Only show tab icon if the user has specifically chosen one
ClosedPublic

Authored by ngraham on Dec 23 2018, 6:26 AM.

Details

Summary

This patch introduces a UI change: tabs only show an icon if it's not
the default one; that is to say, only if the user has actually customized
the icon. There are several reasons for this:

  1. Conceptual: When only non-default profile icons show up in the tab, it does a better job of highlighting the fact that a non-default profile is being used.
  2. Aesthetic: you no longer have the same icon in every tab by default, which looks bad and causes the eye to ignore it over time.
  3. Icon-specific: the utilities-terminal icon doesn't look good against a dark background (https://bugs.kde.org/show_bug.cgi?id=367696)
  4. Code hygiene: reduces a bit of redundancy in the session controller code.

BUG: 401864
CCBUG: 367696
FIXED-IN: 19.04.0

Test Plan

Open multiple tabs and make one of them use a profile with a
non-default icon:

Verify that bell signals still cause tabs to get icons.

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Dec 23 2018, 6:26 AM
Restricted Application added a project: Konsole. · View Herald TranscriptDec 23 2018, 6:26 AM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
ngraham requested review of this revision.Dec 23 2018, 6:26 AM
ngraham edited the test plan for this revision. (Show Details)Dec 23 2018, 6:27 AM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the summary of this revision. (Show Details)
hindenburg edited the summary of this revision. (Show Details)Dec 23 2018, 6:35 PM
hindenburg edited the test plan for this revision. (Show Details)

I don't see any issues - tested w/ monitor for silence/activity works as before. The tabs look odd to me w/o any icon but that's likely just due I'm used to seeing them.

src/SessionController.cpp
1531

For empty strings use QString()

abetts added a subscriber: abetts.Dec 23 2018, 6:55 PM

I have seen implementations like this one before and it makes it so that the tab expands and shrinks when the focus is on the tab because it needs to accommodate for the icon that appears and disappears. Is that the case with this patch?

ngraham updated this revision to Diff 48093.Dec 23 2018, 8:45 PM
ngraham marked an inline comment as done.

Use QString() for an empty string

I have seen implementations like this one before and it makes it so that the tab expands and shrinks when the focus is on the tab because it needs to accommodate for the icon that appears and disappears. Is that the case with this patch?

Yes, when the tab gains an icon, it will become wider to accommodate it. It already has this behavior when it's displaying the name of the running command and that command finishes and is replaced with another, so it's not unprecedented.

Anyone else have any comments on the VD aspect?

ngraham edited the summary of this revision. (Show Details)Thu, Dec 27, 3:17 AM
ngraham edited the summary of this revision. (Show Details)
ndavis accepted this revision.EditedThu, Dec 27, 3:51 AM

While I would like it if the tab could remain a consistent size, regardless of whether or not there is an icon, it doesn't need to be fixed in this revision. What we have here now is a better default because it saves more space.

For anyone that mainly uses only one profile, the icons become useless. For anyone who likes to use multiple profiles, they can easily add icons that have more meaning than the Konsole icon.

This revision is now accepted and ready to land.Thu, Dec 27, 3:51 AM

While I would like it if the tab could remain a consistent size, regardless of whether or not there is an icon, it doesn't need to be fixed in this revision.

+1 . That tabs should stay consistent in size. Because of any size changes not directly triggered by the user you prevent any kind of interaction by spatial memory.

+1 . That tabs should stay consistent in size. Because of any size changes not directly triggered by the user you prevent any kind of interaction by spatial memory.

With this patch, the only way to make a tab gain an icon (and therefore become wider) is to switch it to use a profile that has explicitly been assigned a custom icon. So any width changes are in fact explicitly triggered by the user.

hindenburg accepted this revision.Fri, Dec 28, 3:25 AM
hindenburg edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.