[KDE Usability] Review Request: Allow to hide text for specific icons on toolbars

[David Faure]

> On 2009-10-14 02:58:48, David Faure wrote:
> > Looks good, except for two things:
> > 
> > * Surely a change in kxmlguifactory is missing so that the new xml attribute is read (to honor the setting after restarting the app), isn't it?
> > 
> > * The text of the label is a bit confusing. I had to read the Qt docs to understand that it would hide the text when the icon is shown,
> > and now when icon isn't shown. Hmm. Maybe "Hide text when icon is shown"? "Hide text (except in text only mode)"? Not satisfactory ;)
> > The point is that it seemed illogical to be able to enter a text, and then ask for it to be hidden.
> > Or do you disable the lineedit when the checkbox is checked (because the user knows if he wants TextOnly or IconText for the toolbar anyway)?
> >
> 
> Christoph Feck wrote:
>     ad 1: No further code is required, the XML GUI stuff already saves the element attribute that I add. But I probably should add the "priority" to the dtd/xsd declarations for verifing.
>     
>     ad 2: The problem is that the icon text is only hidden in "Text Alongside Icon" mode, NOT in "Text Below Icon" mode. That's why I cannot disable the line edit when the check box is selected. This is just how the Qt 4.6 "priority" stuff works. It does not hide the text when in "Text Below Icon" mode because the layout would look ugly.
>
> 
> Christoph Feck wrote:
>     Hm, not sure if the label for the checkbox should be more verbose ("Hide text when toolbar is set to Text Alongside Icons mode"?), but a nice tooltip or wtf message should be added to this effect. Suggestions? My english is pretty basic :)
>     
>     Some more information:
>     
>     The reason why I put it there was because people tried to "abuse" the ability to change the icon text to make it invisible, but that did not work (see the comment from Hans on http://kdepepo.wordpress.com/2009/08/10/short-kde-toolbar-texts/ ) If usability people have a different idea for the UI, I am open for suggestions.
>     
>     Another plan was to merge the "Change Icon" and "Change Text" into a single "Customize..." button, where the user was able to change all these attributes. But then, changing an icon would be the fourth level window already, and I scratched that idea.
> 
> David Faure wrote:
>     Sorry I don't understand. I see no code that loads or saves a "priority" attribute right now in kdeui; your patch adds saving of the attribute, but where is the code that uses that attribute?
>     
>     The location of the checkbox seems fine to me, just the wording could be improved.
>     Your more verbose proposal seems right. Maybe simplified a bit like: 
>     "Hide text when toolbar shows text alongside icons"
> 
> Christoph Feck wrote:
>     http://lxr.kde.org/source/KDE/kdelibs/kdeui/xmlgui/kxmlguifactory.cpp#642

Oh, the generic setProperty! Of course ;) I could grep for priority all I wanted, that wouldn't have helped ;)

Looks good to me then.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1847/#review2640
-----------------------------------------------------------


On 2009-10-13 23:44:41, Christoph Feck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1847/
> -----------------------------------------------------------
> 
> (Updated 2009-10-13 23:44:41)
> 
> 
> Review request for kdelibs and usability.
> 
> 
> Summary
> -------
> 
> This replaces the previous text input dialog with a custom dialog that has an additional check box to hide the icon text when mode is "Text Alongside Icons". Requires Qt 4.6 to test.
> 
> 
> This addresses bug 175123.
>     https://bugs.kde.org/show_bug.cgi?id=175123
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kdeui/dialogs/kedittoolbar.cpp 1034928 
>   /trunk/KDE/kdelibs/kdeui/dialogs/kedittoolbar_p.h 1034928 
> 
> Diff: http://reviewboard.kde.org/r/1847/diff
> 
> 
> Testing
> -------
> 
> Compiles and works on current trunk.
> 
> 
> Screenshots
> -----------
> 
> "Change Text..." dialog from toolbar editor
>   http://reviewboard.kde.org/r/1847/s/227/
> 
> 
> Thanks,
> 
> Christoph
> 
>