[Kde-accessibility] Review Request: Focus Tracking in KWin's Zoom Plugin

[Thomas Lübking]

> On Aug. 15, 2012, 5:35 p.m., Martin Gräßlin wrote:
> > First of all: we cannot depend on anything in playground. This review has a push embargo till the library has gone through review.
> > 
> > Overall I dislike the extensive usage of ifdefs. I would prefer if the accessibility feature becomes a mandatory requirement for this effect to build. That is if LibKdeAccessibilityClient is not found the effect does not get build. I think this would make the code much cleaner to read and also to maintain.

I wonder whether the compile time check
a) is required at all
b) a good idea

It looks to me as if the effect would through that lib just hook to some dbus signal - and one does not really have to link some lib for that purpose (that may draw in what not else, notably ati-spi) while and overmore this feature will then likely depend on the presence of some loaded kded module or so, shipped with kaccessibility, rather then the lib presence at compile time, so it actually has to check whether that is installed / loaded - yesno?


- Thomas


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


On Aug. 15, 2012, 3:07 p.m., Amandeep Singh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106041/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2012, 3:07 p.m.)
> 
> 
> Review request for KDE Accessibility, kwin, Frederik Gladhorn, Sebastian Sauer, and Luboš Luňák.
> 
> 
> Description
> -------
> 
> This patch is for KWin, makes the focus-tracking feature of KWin work with applications. This makes KDE more accessible.
> A new accessibility client library "libkdeaccessibilityclient" (which can be found here: https://projects.kde.org/projects/playground/accessibility/libkdeaccessibilityclient) is also to be added in kde-support.
> 
> The focus-tracking feature will use the new library to get the accessibility information from at-spi.
> 
> Adjustments in CMakeLists have been made to add the library as an optional dependency, which if found enables the focus-tracking option in Zoom Plugin.
> And in absence of library, the option is removed.
> 
> 
> Diffs
> -----
> 
>   kwin/effects/CMakeLists.txt 29accb1 
>   kwin/effects/zoom/CMakeLists.txt 36e11ac 
>   kwin/effects/zoom/focustrackconfig.h.in PRE-CREATION 
>   kwin/effects/zoom/zoom.h d809b21 
>   kwin/effects/zoom/zoom.cpp b275b1e 
>   kwin/effects/zoom/zoom_config.cpp 3edd10c 
> 
> Diff: http://git.reviewboard.kde.org/r/106041/diff/
> 
> 
> Testing
> -------
> 
> I tested KWin's new feature working with KWrite and Konsole.
> 
> 
> Thanks,
> 
> Amandeep Singh
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-accessibility/attachments/20120815/8bc8e6c9/attachment.html>