Review Request 119428: Port Dolphin accessibility to Qt 5

Frank Reininghaus frank78ac at googlemail.com
Mon Jul 28 16:47:20 BST 2014


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


Thanks for working on this Frederik! Making a11y work again and removing many lines of code at the same time is fantastic!

I cannot really say much about the code since my accessibility knowledge is rather limited. If nobody else has any comments, I'd say that you should go ahead and push this in a few days.

I do have a few comments about the coding style though: 

1. Use {...} even for one-line statements after if, for, while, etc.
2. In the existing Dolphin code (also in the files which are modified by this patch), there is no space in front of "*" and "&", but a space behind these characters.


dolphin/src/kitemviews/kitemlistviewaccessible.cpp
<https://git.reviewboard.kde.org/r/119428/#comment44149>

    Use {...} even for single-line statements after "if".



dolphin/src/kitemviews/kitemlistviewaccessible.cpp
<https://git.reviewboard.kde.org/r/119428/#comment44150>

    space after "x,"


- Frank Reininghaus


On July 26, 2014, 11:54 a.m., Frederik Gladhorn wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119428/
> -----------------------------------------------------------
> 
> (Updated July 26, 2014, 11:54 a.m.)
> 
> 
> Review request for Dolphin, Frank Reininghaus, Sebastian Sauer, and Jeremy Whiting.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Port the accessibility implementation for the custom list views to Qt 5. Lose 90 lines of code on the way :)
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistviewaccessible.cpp d9ddd58 
>   dolphin/src/CMakeLists.txt cc358a2 
>   dolphin/src/kitemviews/kitemlistview.cpp d35dd89 
>   dolphin/src/kitemviews/kitemlistviewaccessible.h c2213cd 
> 
> Diff: https://git.reviewboard.kde.org/r/119428/diff/
> 
> 
> Testing
> -------
> 
> Ran dolphin a few times. It works nicely with Orca.
> Make sure to run "gsettings set org.gnome.desktop.a11y.applications screen-reader-enabled true" once before testing, then Orca should just work and read Dolphin :)
> All the tedious setup from Qt 4 is gone and the code quite a bit cleaner all in all.
> 
> 
> Thanks,
> 
> Frederik Gladhorn
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140728/df0fbf7c/attachment.htm>


More information about the kfm-devel mailing list