Bug 332629 - Toolbar buttons for next and previous tabs are wordy
Summary: Toolbar buttons for next and previous tabs are wordy
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: general (show other bugs)
Version: 4.11.5
Platform: Mint (Ubuntu based) Linux
: NOR minor
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords: junior-jobs
Depends on:
Blocks:
 
Reported: 2014-03-26 14:17 UTC by Brock McNuggets
Modified: 2014-04-29 19:57 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.14.0


Attachments
correct the action caption (1.12 KB, patch)
2014-03-31 19:45 UTC, Mario Scheel
Details
proposed solution (comment #2) (1.11 KB, patch)
2014-04-25 23:24 UTC, Renato Atilio
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brock McNuggets 2014-03-26 14:17:32 UTC
Activate Next Tab / Activate Previous Tab
Wordy: no need for word "Activate". User can re-word them himself but the default should fit on toolbars better.

Reproducible: Always
Comment 1 Frank Reininghaus 2014-03-26 14:53:27 UTC
Thanks for the report. This text is not Dolphin-specific. It comes from 
kstandardshortcut.cpp in kdelibs/kdeui.
Comment 2 Christoph Feck 2014-03-26 22:20:32 UTC
This is about the action texts, not the keyboard shortcuts. dolphinmainwindow.cpp::1635 says:

    KAction* activateNextTab = actionCollection()->addAction("activate_next_tab");
    activateNextTab->setText(i18nc("@action:inmenu", "Activate Next Tab"));

I suggest to setIconText() a shorter version of the string for the tool bar in addition to setText() the longer version for the menu.
Comment 3 Frank Reininghaus 2014-03-27 10:44:23 UTC
Thanks for the info and sorry for the mistake, Christoph. I first thought that the text is picked up by applications from kstandardshortcut.cpp, where I quickly found it, but you are right, of course.

(In reply to comment #2)
> I suggest to setIconText() a shorter version of the string for the tool bar
> in addition to setText() the longer version for the menu.

Yes, good idea. This looks like it could be a good Junior Job for potential new contributors.
Comment 4 Mario Scheel 2014-03-31 19:45:47 UTC
Created attachment 85876 [details]
correct the action caption

This patch removes "Activate" from the action "Previous Tab" and "Next Tab".
Comment 5 Emmanuel Pescosta 2014-03-31 19:56:54 UTC
@Mario Scheel
Thanks for the patch, looks good! :)

Can you please adjust your patch so that it uses the long text in the menu and the short text in the toolbar? - See comment #2 for additional information.

And then you can post your patch on reviewboard http://git.reviewboard.kde.org - example review request https://git.reviewboard.kde.org/r/117209/ - or just upload your new patch here ;)
Comment 6 Renato Atilio 2014-04-25 23:23:26 UTC
I'm attaching a diff with the proposed solution. I'm going to create a review request, but I'd like to know first if icons are desired for these actions.

I tried to find the left and right arrows used on QTabBar but it is linked to QToolButton. Would something like this be useful (or any other icons):
activatePrevTab->setIcon(KIcon("arrow-left"));
activateNextTab->setIcon(KIcon("arrow-right"));
?

Probably using QApplication::isRightToLeft() to determine the arrow direction.
Comment 7 Renato Atilio 2014-04-25 23:24:09 UTC
Created attachment 86270 [details]
proposed solution (comment #2)
Comment 8 Renato Atilio 2014-04-26 18:20:15 UTC
Well, I've submitted a review request strictly with the expected change.

https://git.reviewboard.kde.org/r/117794/
Comment 9 Frank Reininghaus 2014-04-29 19:57:06 UTC
Git commit 03f7f20b9ffa3cd1b9e090b191630888a6a0f99c by Frank Reininghaus, on behalf of Renato Atilio.
Committed on 29/04/2014 at 19:54.
Pushed by freininghaus into branch 'master'.

Change the icon text for Previous and Next toolbar buttons

In addition to the current long text for previous and next toolbar
buttons ("Activate Previous/Next Tab"), this commit adds shorter icon
texts for them to be used only on the toolbar ("Previous/Next Tab").
REVIEW: 117794
FIXED-IN: 4.14.0

M  +2    -0    dolphin/src/dolphinmainwindow.cpp

http://commits.kde.org/kde-baseapps/03f7f20b9ffa3cd1b9e090b191630888a6a0f99c