Review Request: Improving the Accessibility Interface for Dolphin

[Frank Reininghaus]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106555/#review19396
-----------------------------------------------------------

Ship it!


Thanks for the patch!

> Hi Frank. Sorry for unintended damage.

All right, apology accepted :-) Just remember to format the commits in a working branch nicely before pushing large changes in the future. If this seems to be too much work, which it usually is, I think it's better to push such changes in a single commit rather than including the full history, unless the history may be helpful for understanding the feature (which was IMHO not the case here).

Using friends rather than giving public access to the layouter looks like an improvement to me. I still think that there might be corner cases where the row/column calculation could go wrong, but fixing this is a job for me, I'll try to remember to look into it before we enter the beta phase.

This is OK to go in from my point of view (after considering my comments below). The only thing that's not quite clear to me is why you propose to remove an assert which actually looks quite reasonable at first sight?


dolphin/src/kitemviews/kitemlistview.cpp
<http://git.reviewboard.kde.org/r/106555/#comment15342>

    Add spaces after each comma.



dolphin/src/kitemviews/kitemlistviewaccessible.cpp
<http://git.reviewboard.kde.org/r/106555/#comment15345>

    Why is the assert not valid any more?



dolphin/src/kitemviews/kitemlistviewaccessible.cpp
<http://git.reviewboard.kde.org/r/106555/#comment15344>

    Put spaces next to the "=" here and in the line below.



dolphin/src/kitemviews/kitemlistviewaccessible.cpp
<http://git.reviewboard.kde.org/r/106555/#comment15343>

    I'd prefer to leave the 'child' as it is and add a Q_UNUSED(child) to suppress a compiler warning.


- Frank Reininghaus


On Sept. 24, 2012, 8:03 a.m., Amandeep Singh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106555/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2012, 8:03 a.m.)
> 
> 
> Review request for Dolphin, Frederik Gladhorn and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Hi Frank. Sorry for unintended damage. I have removed the layouter function from public in this diff. Also had to make other changes to make the AccessibleInterface work with Orca as well.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistview.h 2c30c6f 
>   dolphin/src/kitemviews/kitemlistview.cpp 580cf5b 
>   dolphin/src/kitemviews/kitemlistviewaccessible.cpp 6ca9cc8 
> 
> Diff: http://git.reviewboard.kde.org/r/106555/diff/
> 
> 
> Testing
> -------
> 
> Tried it with Orca and KMag
> 
> 
> Thanks,
> 
> Amandeep Singh
> 
>

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