[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: Allow to hide text for specific icons on toolbars
From:       "Christoph Feck" <christoph () maxiom ! de>
Date:       2009-10-15 11:16:34
Message-ID: 20091015111634.12064.39189 () localhost
[Download RAW message or body]



> 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"

http://lxr.kde.org/source/KDE/kdelibs/kdeui/xmlgui/kxmlguifactory.cpp#642


- Christoph


-----------------------------------------------------------
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
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]