Edit Profile Dialog UI redesign
Needs ReviewPublic

Authored by mglb on Thu, Nov 29, 8:54 PM.

Details

Reviewers
None
Group Reviewers
Konsole
VDG
Summary

Overall changes

  • Use a dialog with category buttons on the left, which is used in most KDE applications
  • Apply KDE HIG as much as possible
  • Align layout columns in multiple group boxes
  • Move some settings to another groups

General page

Profile name and icon, and settings related to session/application
initialization.

  • Move "Show hint for terminal size after resizing" to Appearance page
  • Move "Dim the colors when the window loses focus" to Appearance page

Tabs page, rename tab dialog

Tabs settings

  • Only minor UI changes

Appearance page


Settings related to basic appearance.

  • Add additional tabs
    • Cursor - cursor settings from Advanced page
    • Miscellaneous
      • Add "Line spacing" from Advanced page
      • Add previously missing terminal margins and terminal center settings (4 years old config-only feature)
      • Add "Show hint for terminal size after resizing" from General page
      • Add "Dim the colors when the window loses focus" from General page
  • Use customized font selection dialog
    • Show all printable ASCII characters and look-alike character sets as a preview
    • Live preview for changes in the dialog
  • Move "Show all fonts" to the font selection dialog
  • Remove "text size" (it is replaced with live preview in the font dialog)
  • Add live preview for cursor settings
  • Add live preview for "Line spacing"

Scrolling page, history size dialog

Settings related to scrolling and history.

  • Replace popping-in warning frames in "Scrollback" group with warning buttons which show floating warning after click. The controls does not change position anymore when switching the scrollback options. Applies also to history size dialog.
  • Replace scrollbar "hide"/"show on left side"/"show on right side" options with "visible" checkbox and "show on left side"/"show on right side" options enabled after checking the checkbox.

Keyboard page

  • Removed redundant group box

Mouse page


  • Shorten "Characters considered part of a word..." label
  • Replace "triple-click selects" drop-down with option buttons
  • Split settings to "Text interaction" and "Miscellaneous" tabs
  • Use monospace font for "Word characters" text input

Advanced page

More advanced settings or settings regular user don't care about.

  • Replace "Show URL hints when these keys are pressed" checkboxes with toggle buttons which are easier to associate visually with hardware keys
  • Move "Line spacing" to Appearance tab
  • Move cursor settings to Appearance tab
  • Show "Default character encoding" value directly on drop-down button

Preview for: breeze (dark colors), Oxygen, QtCurve

Test Plan
  • Check visually with light/dark color scheme, Breeze, Fusion, Oxygen, QtCurve widget styles, normal/large font, QT_SCALE_FACTOR set to 1 and 2
  • Change every possible control to check UI logic
  • Change as much settings as possible and see if they are applied

Diff Detail

Repository
R319 Konsole
Branch
arc/Edit-Profile-Dialog-UI-redesign (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 5686
Build 5704: arc lint + arc unit
mglb requested review of this revision.Thu, Nov 29, 8:54 PM
mglb created this revision.

!!!!

I had this on my-to-do list, but this looks amazing! A super duper humongous improvement! I will test this out and formally review soon!

This is just awesome. I have some inline comments to really polish the heck out of it:

src/EditProfileAdvancedPage.ui
176

In general, we try not to do
<thing> [x] Enabled.
Instead, do
[x] <thing>

If this would be awkward because there are no left labels, try to group them together and give the first one a "miscellaneous" or "general" label. Here's how it would look for this:

Miscellaneous: [ ] Reverse URL hint numbering
               [ ] Allow blinking text
               [ ] Flow control
               [ ] Bi-directional text rendering
src/EditProfileAppearancePage.ui
311

This could probably be shortened to just "Match current character"

398

It might not be the worst thing in the world to just remove the right label for this, and end up with:

Blinking: [ ]
608

This could probably skip the group box and be:

Window: [x] Show hint for terminal size after resizing
        [ ] Dim colors when inactive
src/EditProfileScrollingPage.ui
156

I think this would actually be simpler and better as a three-item radio button:

Scrollbar position: (o) Right side
                    ( ) Left side
                    ( ) Hidden
src/HistorySizeWidget.ui
38–40

We could probably remove the word "scrollback" from these strings, since the radio buttons' left label already uses it. So we would have:

Scrollback: (o) Fixed size
            ( ) Unlimited
            ( ) none

For some reason in this revision if I open the settings then click another page and go back to the general page, When i click the environment edit button the pop up gets opened several times in a row. I've added a gif showing the issue.

mglb updated this revision to Diff 46816.Mon, Dec 3, 11:48 PM

Thanks!

All suggestions applied. However, I think the "Blinking" checkbox looks a bit weird without the right label:

Like, something is missing there visually.

@rizzitello thanks, bug fixed. Signal-slot connections were created on each tab change.

mglb updated this revision to Diff 46817.Mon, Dec 3, 11:53 PM
mglb edited the summary of this revision. (Show Details)
  • Remove unnecessary line from description
  • Add: Use monospace font for "Word characters" text input
mglb marked 6 inline comments as done.Mon, Dec 3, 11:55 PM

Wow, that's impressive. Thanks a lot for working on this - let me look at it further.

Does anyone else get this on the terminal that you started Konsole?

qt.svg: link y hasn't been detected!

ngraham added subscribers: mart, hein.Tue, Dec 4, 3:03 PM

Thanks so much!

Replace popping-in warning frames in "Scrollback" group with warning buttons which show floating warning after click. The controls does not change position anymore when switching the scrollback options. Applies also to history size dialog.

This one one change I'm a little iffy about. I can understand that some people don't like KMessageWidgets because they reposition content, but I think it highlights the need for a new style of pop-up that appears over the view and looks visually connected to a particular widget. I believe @hein and @mart have discussed this in the past.

mglb added a comment.Wed, Dec 5, 5:44 AM

New widget would be nice. The information here is not that important (for advanced user) and is not an indicator (where kmessagewidget is perfect) but more like an one-time hint. The warning icon indicates that the option should be used with caution, or that the configuration is somehow risky. Someone who cares about it can just click and read a message.

Current implementation could stay here until the new widget is made. It will also work as an use case and one of possible designs.

Is the discussion available somewhere, or was it on irc/telegram?

loh.tar added a subscriber: loh.tar.Wed, Dec 5, 6:27 AM

Just a hint/question from someone who didn't try this particular patch.

What happens when the screen has a limited height? Can/will the dialog shrink to fit in?

There was a similar change sometime in the past (sadly I can't remember ATM where/what, sorry) which caused on my loved Netbook (1024x600) a very bad experience. You had to move the window by Alt-Mouse-Drag to view/access lower widgets.

Furthermore have these dialogs an issue when the text below the left icon row is too long, the text is broken then (not full readable) and you can't adjust the width of this column manually.

emateli added a subscriber: emateli.Wed, Dec 5, 4:58 PM

Would be great if the "Line spacing" and "Margins" checkboxes were aligned.

In D17244#371489, @mglb wrote:

New widget would be nice. The information here is not that important (for advanced user) and is not an indicator (where kmessagewidget is perfect) but more like an one-time hint. The warning icon indicates that the option should be used with caution, or that the configuration is somehow risky. Someone who cares about it can just click and read a message.

Yeah, that makes sense.

Current implementation could stay here until the new widget is made. It will also work as an use case and one of possible designs.

Agreed.

Is the discussion available somewhere, or was it on irc/telegram?

I think it was only on real-time chat, but @hein might remember if there was something that was written down in a more permanent location.