[KDE Usability] Review Request 126467: Add Case Sensitive Sorting to Dolphin

[Emmanuel Pescosta]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126467/#review91186
-----------------------------------------------------------



Some final nitpicks, looks pretty good otherwise! :)


kitemviews/kfileitemmodel.h (line 182)
<https://git.reviewboard.kde.org/r/126467/#comment62209>

    make it private (and maybe add a short description from where it loads the settings and what it does)



kitemviews/kfileitemmodel.cpp (lines 61 - 62)
<https://git.reviewboard.kde.org/r/126467/#comment62208>

    Please keep that empty line between sorting and dir lister, to improve the readability of the code.



kitemviews/kfileitemmodel.cpp (lines 785 - 794)
<https://git.reviewboard.kde.org/r/126467/#comment62211>

    use a switch statement instead (to make it more readable you could also add a using Choice = GeneralSettings::EnumSortingChoice; in front of the switch statement, and then simply use Choice::...)



settings/dolphin_generalsettings.kcfg (line 97)
<https://git.reviewboard.kde.org/r/126467/#comment62212>

    I think that the prefix can be removed, because it doesn't add any useful information. (If you want to use the prefix, then please use the full enum name instead and remove Sorting in all choices)



settings/general/behaviorsettingspage.cpp (lines 149 - 155)
<https://git.reviewboard.kde.org/r/126467/#comment62214>

    Maybe make use of Choice = GeneralSettings::EnumSortingChoice; to make it easier to read ;)



settings/general/behaviorsettingspage.cpp (lines 160 - 166)
<https://git.reviewboard.kde.org/r/126467/#comment62213>

    use a switch statement instead (to make it more readable you could also add a using Choice = GeneralSettings::EnumSortingChoice; in front of the switch statement, and then simply use Choice::...)


- Emmanuel Pescosta


On Jan. 16, 2016, 5:11 p.m., arnav dhamija wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126467/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2016, 5:11 p.m.)
> 
> 
> Review request for Dolphin, KDE Usability and Emmanuel Pescosta.
> 
> 
> Bugs: 148550
>     https://bugs.kde.org/show_bug.cgi?id=148550
> 
> 
> Repository: dolphin
> 
> 
> Description
> -------
> 
> See also: https://git.reviewboard.kde.org/r/112330/
> 
> This patch was redone because of the transition to KF5.
> 
> This patch adds an option for case sensitive sorting in Dolphin under General Settings by updating the Case Sensitivity of the QCollator in kfileitemmodel.cpp as with the user's selection. Much of what I have done has been based off the code for the Natural Sorting option.
> 
> However I have made a few observations of the results:
> 1) Toggling the Natural Sorting or the Case Sensitive Sorting checkbox in General Settings will not update the sorting of the files in a directory until Dolphin is restarted (I guess this is a bug?).
> 2) If Natural Sorting AND Case Sensitive Sorting are enabled together, Dolphin will sort the files in Natural Sorting using Case Insensitive mode. This is probably a bug on my part - so I would be glad to have a more experienced programmer take a look at it.
> 
> After some feedback from the usability team, I have changed the GUI of the sorting options menu a bit:
> 
> <RadioButton>Enable Extra Sorting Features
> <RadioButton>Natural Sorting
> <RadioButton>Case Sensitive Sorting
>         
> An example of this can be seen in the screenshot attached with this review request.
> 
> EDIT: it was suggested in the comments to use 3 radio buttons for the sorting options as all 3 are mutually exclusive. This change is reflected in the 2nd screenshot (on Unity Ubuntu 15.10). Finally, I used an Enum to switch between the 3 sorting modes and updated the KConfig file accordingly.
> 
> EDIT2: Bumping this for another review and whether it is ready to be shipped.
> 
> 
> Diffs
> -----
> 
>   kitemviews/kfileitemmodel.h 167f508 
>   kitemviews/kfileitemmodel.cpp 5f6fed0 
>   settings/dolphin_generalsettings.kcfg 9ff14d1 
>   settings/general/behaviorsettingspage.h 6e49169 
>   settings/general/behaviorsettingspage.cpp 093a1f4 
> 
> Diff: https://git.reviewboard.kde.org/r/126467/diff/
> 
> 
> Testing
> -------
> 
> manual
> 
> 
> File Attachments
> ----------------
> 
> Idea for Dolphin General Settings behavior tab
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/12/23/d8e9eab1-1c9e-4cac-84e6-11ad10c077e9__dolphin_settings.png
> Screenshot from 2015-12-23 22-21-24.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/12/23/a04136b1-b7fa-45c4-a2ec-a8d8fd3e8625__Screenshot_from_2015-12-23_22-21-24.png
> 
> 
> Thanks,
> 
> arnav dhamija
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20160116/20237407/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-usability mailing list
kde-usability at kde.org
https://mail.kde.org/mailman/listinfo/kde-usability