[Kde-accessibility] Review Request 121210: Provide an accessible name for KLed

[Christoph Feck]

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


I think the updateAccessibleName() should also be called from the constructors? Otherwise, the initial name (for which even a constructor exists), is never announced.


src/kled.cpp
<https://git.reviewboard.kde.org/r/121210/#comment49502>

    The capitalization "Led" is used in Qt coding style, but the correct term is "LED". Please use all caps in user-visible strings.



src/kled.cpp
<https://git.reviewboard.kde.org/r/121210/#comment49504>

    Please use kdelibs coding style:
    
    if () {
        ...
    } else {
        ...
    }
    
    Or, in this case, use a ?: operator, as is used elsewhere in this file.



src/kled.cpp
<https://git.reviewboard.kde.org/r/121210/#comment49503>

    Please do not call this here, this function is called for any changes affecting the look.
    
    There are only two functions which change the state, toggle() and setState(). Move it there.


- Christoph Feck


On Nov. 22, 2014, 4:32 p.m., José Millán Soto wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121210/
> -----------------------------------------------------------
> 
> (Updated Nov. 22, 2014, 4:32 p.m.)
> 
> 
> Review request for KDE Accessibility and KDE Frameworks.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> -------
> 
> This patch will provide a default accessible name for KLed. That name will only say that the widget is a led and the status of the led. If another accessible name has been assigned, it will not be overwritten.
> The accessible name will allow accessible tools (such as screen readers) to provide some infomation about the widget.
> 
> 
> Diffs
> -----
> 
>   src/kled.h eeb1209 
>   src/kled.cpp 9788fc2 
> 
> Diff: https://git.reviewboard.kde.org/r/121210/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> José Millán Soto
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-accessibility/attachments/20141122/45712d9d/attachment.html>