Bug 411223 - Dolphin’s sort order chooser human-readable text
Summary: Dolphin’s sort order chooser human-readable text
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: general (show other bugs)
Version: 19.08.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-23 23:37 UTC by Gabriel Fernandes
Modified: 2019-09-09 13:17 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 19.08.2


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gabriel Fernandes 2019-08-23 23:37:14 UTC
SUMMARY
Bug introduced in this commit https://phabricator.kde.org/D22006

STEPS TO REPRODUCE
1. Choose one type of sorting (by name, for example)
2. Click again the same sorting (or use shortcut)

OBSERVED RESULT
Short order will be messed up, and won't display the one currently in use.
You can observe this outcome in various places, like in the right-click and then sort-by, or in the toolbar.
None of the two available sort order (A-Z and Z-A, in this example) will be selected.


EXPECTED RESULT
There should be always one of the two sort order selected (or active, graphical hint of the one in use).


SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: 
(available in About System)
KDE Plasma Version: 
KDE Frameworks Version: 
Qt Version: 

ADDITIONAL INFORMATION
Sadly with this new commit we loose the ability to have just one toolbar icon that toggle the reversed order, it were so useful, specially when set a shortcut to toggle reversed, this new change requires two things in the toolbar, and two shortcuts (if you wanna fast switches between reversed and non-reversed). It would be really appreciated if the old toolbar that toggles and possibility to set one shortcut for toggle were back.
Comment 1 Nate Graham 2019-08-24 15:10:06 UTC
Can you attach a screen recording that shows the problem? I'm having a hard time understanding, and the Steps To Reproduce aren't detailed enough (for example, how are you choosing the sorting? What are you clicking on?)
Comment 2 Gabriel Fernandes 2019-08-24 19:08:58 UTC
sure, https://youtu.be/TKQrFNcf9Qw
Comment 3 Nate Graham 2019-08-24 19:42:20 UTC
Thanks for the video, that's super helpful. Can confirm. Hopefully not too hard to fix.
Comment 4 Nate Graham 2019-08-24 20:38:57 UTC
Filed a patch: https://phabricator.kde.org/D23412
Comment 5 Gabriel Fernandes 2019-08-24 23:47:23 UTC
great to hear
Comment 6 Gabriel Fernandes 2019-09-07 18:54:38 UTC
I thought it would land in Dolphin Version 19.08.1
I tried the patch myself, it seems to work
Comment 7 Nate Graham 2019-09-07 19:58:53 UTC
Still in patch review, I'm afraid.
Comment 8 Gabriel Fernandes 2019-09-07 21:01:14 UTC
But it seems to be missing a parenthesis at the end of the for, in the diff. I had added one before the curly at the end in my local build.
for (QAction *groupAction : qAsConst(m_sortByActions) ->)<- {
Comment 9 Nate Graham 2019-09-09 13:17:34 UTC
Git commit 60d6a3bdbcd44360e1c6ae1c82239ecf6d60ded7 by Nate Graham.
Committed on 09/09/2019 at 13:17.
Pushed by ngraham into branch 'Applications/19.08'.

Fix ascending/descending choosers getting unchecked when re-selecting the same sort order

Summary:
When you select the same sort order that's already selected, the currently-checked
sort order description (the human-readable ascending/descending items) gets unchecked
in `slotSortTriggered()` yet the ascending/descending items items only get checked in
`slotSortOrderChanged()`. Because the order hasn't gotten changed, neither one gets
checked again.

This patch fixes the problem by not unchecking them in the first place.
FIXED-IN: 19.08.2

Test Plan:
1. Right-click > Sort By > Click the currently-selected sort order
2. Right-click > Sort By > See that the item for the current ascending/descending setting has not been changed

Reviewers: elvisangelaccio, #dolphin

Reviewed By: elvisangelaccio, #dolphin

Subscribers: broulik, meven, kfm-devel

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D23412

M  +3    -2    src/views/dolphinviewactionhandler.cpp

https://commits.kde.org/dolphin/60d6a3bdbcd44360e1c6ae1c82239ecf6d60ded7