Bug 1191112 - RFE: Indicate that <tab> is used to navigate
Summary: RFE: Indicate that <tab> is used to navigate
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: powertop
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jaroslav Škarvada
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-02-10 13:32 UTC by Pierre-YvesChibon
Modified: 2015-02-27 09:58 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-02-11 20:15:44 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Proposed patch (431 bytes, patch)
2015-02-11 12:42 UTC, Jaroslav Škarvada
no flags Details | Diff

Description Pierre-YvesChibon 2015-02-10 13:32:21 UTC
Description of problem:
Since the change in the UI of powertop I was not able to actually make it work.
In the old UI, one would use the arrow keys to navigate through the settings and tune them.
With the new UI, using the arrow key in the front page does nothing, it just moves the page and the user has to use the <tab> key to arrive to the `Tunables` tab where action can be taken.

I learned this last week-end, this new behavior was really not easily discoverable for me.

Maybe in the bottom bar where is mentioned `<ESC> Exit` could be also mentioned that <tab> is used to navigate through the different pages/panels/tabs.


Version-Release number of selected component (if applicable):
powertop-2.7-2.fc21.x86_64


How reproducible:
Always (if you don't know it)

Steps to Reproduce:
1. Start powertop
2. Ignore that <tab> can be used to navigate
3. Try using the arrow keys as you did before the new UI

Actual results:
You cannot change any settings, the whole page moves

Expected results:
An information message?
Or more simply, mentioning at the bottom of the page that one should use <tab> to navigate

Additional info:

Comment 1 Pierre-YvesChibon 2015-02-10 13:56:16 UTC
Offer a fix upstream: https://github.com/fenrus75/powertop/pull/19

Comment 2 Jaroslav Škarvada 2015-02-10 17:13:23 UTC
Thanks for the report and patch.

Please also add "Shift + Tab".

I will apply your patch downstream.

Comment 3 Jaroslav Škarvada 2015-02-10 17:15:35 UTC
(In reply to Jaroslav Škarvada from comment #2)
> Please also add "Shift + Tab".
> 
I.e:
+ mvwprintw(bottom_line, 0,0, _("<ESC> Exit | <TAB> / <Shift + TAB> Navigate |"));

or similarly.

Comment 4 Pierre-YvesChibon 2015-02-10 17:49:49 UTC
Updated on github.

Btw, are you on the upstream mailing-list?

It seems to be the place to submit patches but I am not subscribed and do not feel like doing so just for a patch.

Comment 5 Jaroslav Škarvada 2015-02-11 12:20:00 UTC
(In reply to Pierre-YvesChibon from comment #4)
> Updated on github.
>
Thanks.

> Btw, are you on the upstream mailing-list?
> 
Yes.

> It seems to be the place to submit patches but I am not subscribed and do
> not feel like doing so just for a patch.
>
In the past Arjan also merged pull requests from github without problem, I don't know whether Alexandra do the same. But no problem, I will forward your patch there. At least it will get more attention. IIRC it was also possible to send to the list without registration (moderated).

Comment 6 Jaroslav Škarvada 2015-02-11 12:42:05 UTC
Created attachment 990432 [details]
Proposed patch

(In reply to Pierre-YvesChibon from comment #4)
I think the patch can be still improved. This mod could ease the translation, please update the pull request. I haven't sent the patch upstream yet, feel free to send it yourself, or let me know if you need forward.

Comment 7 Pierre-YvesChibon 2015-02-11 13:14:13 UTC
Thanks for the change, I had started with something like this but I was not sure the two `%s` were valid (and how to specify the two arguments), so instead I used the same approach as elsewhere in the code:

src/tuning/tuning.cpp:  create_tab("Tunables", _("Tunables"), w, _(" <ESC> Exit | <Enter> Toggle tunable | <r> Window refresh"));

Comment 8 Pierre-YvesChibon 2015-02-11 13:15:13 UTC
Shall I just incorporate your changes into a commit in my branch, or do you want to send me a git format-patch (thus keeping the attribution to you :))

Comment 9 Jaroslav Škarvada 2015-02-11 13:20:12 UTC
(In reply to Pierre-YvesChibon from comment #7)
> Thanks for the change, I had started with something like this but I was not
> sure the two `%s` were valid (and how to specify the two arguments), so
> instead I used the same approach as elsewhere in the code:
> 
> src/tuning/tuning.cpp:  create_tab("Tunables", _("Tunables"), w, _(" <ESC>
> Exit | <Enter> Toggle tunable | <r> Window refresh"));

Of course, the coding style of the project is probably inconsistent (as usually :), but the proposed change is definitely better for translators.

Comment 10 Jaroslav Škarvada 2015-02-11 13:23:38 UTC
(In reply to Pierre-YvesChibon from comment #8)
> Shall I just incorporate your changes into a commit in my branch, or do you
> want to send me a git format-patch (thus keeping the attribution to you :))

Just incorporate the change. I send dozens patches daily to different projects, so no need to attribute simple improvement of UI improving one-liner (or nearly one-liner :)

Comment 11 Pierre-YvesChibon 2015-02-11 14:02:49 UTC
Thanks, pull-request updated! :)

Comment 12 Pierre-YvesChibon 2015-02-26 20:59:45 UTC
Upstream is asking on github for the patch to be sent to the list. Is this something you could do?

Comment 13 Jaroslav Škarvada 2015-02-27 09:14:57 UTC
(In reply to Pierre-YvesChibon from comment #12)
> Upstream is asking on github for the patch to be sent to the list. Is this
> something you could do?

NP, forwarded.

Comment 14 Pierre-YvesChibon 2015-02-27 09:58:27 UTC
Thanks!


Note You need to log in before you can comment on or make changes to this bug.