Use logically correct icon for direction arrows
ClosedPublic

Authored by shubham on Feb 20 2019, 2:34 PM.

Details

Test Plan

Before:


After:

Diff Detail

Repository
R223 Okular
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
shubham created this revision.Feb 20 2019, 2:34 PM
Restricted Application added a project: Okular. · View Herald TranscriptFeb 20 2019, 2:34 PM
Restricted Application added a subscriber: okular-devel. · View Herald Transcript
shubham requested review of this revision.Feb 20 2019, 2:34 PM
shubham edited the test plan for this revision. (Show Details)Feb 20 2019, 2:36 PM
shubham added a reviewer: VDG.
shubham retitled this revision from Use logically correct icon to Use logically correct icon for direction arrows.
shubham edited the test plan for this revision. (Show Details)Feb 20 2019, 2:38 PM
ngraham accepted this revision.Feb 20 2019, 7:02 PM
ngraham added a reviewer: Okular.
ngraham added a subscriber: ngraham.

Makes sense to me. Seems to make more sense when using an RTL layout too. Okular folks, are you good with this?

This revision is now accepted and ready to land.Feb 20 2019, 7:02 PM

Up and down arrows are equivalent to arrow key behavior, both in the view area and the PageNumberEdit. It also fits better to documents with RTL page layout (mangas) (if there are such documents).

It’s just irritating next to the PageNumberEdit, where “Up” doesn’t mean “Increment Page Number”. (Although scroll wheel “Up” does mean “Increment” there.)

I think up and down is an improvement. Is this also for the tool bar widget?

aacid added a subscriber: aacid.Feb 21 2019, 6:32 PM

Personally i don't like it given than it is a very horizontal layout.

To mhe mental image here is page turning.

But Adobe reader has ↑↓ buttons, so what do i know

Personally i don't like it given than it is a very horizontal layout.

To me, the mental image here is page turning.

If the document view on the screen were also doing page turning, I would agree with you. But Okular uses up-and-down scrolling instead, to which these arrows map better IMHO.

aacid added a comment.Feb 21 2019, 6:45 PM

Personally i don't like it given than it is a very horizontal layout.

To me, the mental image here is page turning.

If the document view on the screen were also doing page turning, I would agree with you. But Okular uses up-and-down scrolling instead, to which these arrows map better IMHO.

Note that you can have a N column view so the current page will go to the left or right N-1 times of every N clicks.

What's the call?

I feel like this better suits the common cases. I say go for it unless @aacid wants to formally request changes.

@aacid Do you have anything to say?

Isn't this broken when using a RTL layout?

Isn't this broken when using a RTL layout?

Yes, now that you say it... The test for layoutDirection() should be removed.

shubham updated this revision to Diff 52727.Feb 27 2019, 2:31 PM

remove check for layout Direction

@aacid @ngraham Is this good to go now?

I think so.

aacid added a comment.Mar 5 2019, 11:53 PM

I already said that i personally don't like it.

Nathan, Andy and David seem to like it (even though none answered about how this works visually when on N columns mode where it doesn't actually move the focused page up/down but left/right)

You can either trust them to know better than me how pretty things look like (which is probably true) or if you want you can try to find more opinions.

shubham updated this revision to Diff 53789.Mar 13 2019, 1:54 PM

Rebase on master

This revision was automatically updated to reflect the committed changes.