On Tue, 9 May 2023 15:08:30 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

>> Ambarish Rapte has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'master' into a11y-8284542-CheckBoxTreeItem
>>  - Review Comment: Add enum ToggleState
>>  - Add CheckBoxTreeItem role and TOGGLE_STATE attribute
>
> modules/javafx.controls/src/main/java/javafx/scene/control/cell/CheckBoxTreeCell.java
>  line 495:
> 
>> 493:                     state = 2;
>> 494:                 } else if (checkBox.isSelected()) {
>> 495:                     state = 1;
> 
> I recommend using platform-independent constants here (possibly an enum as 
> discussed earlier).

Added a new enum `ToggleState`.

> modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinAccessible.java 
> line 1588:
> 
>> 1586:         if (isDisposed()) return 0;
>> 1587:         if (getAttribute(ROLE) == AccessibleRole.CHECK_BOX_TREE_ITEM) {
>> 1588:             return (int)getAttribute(TOGGLE_STATE);
> 
> I recommend mapping the return value of `getAttribute(TOGGLE_STATE)`, which 
> should be a platform-independent value, to one of the three Windows-specific 
> values. Otherwise you are making an assumption that might not hold for other 
> platforms (e.g., macOS).

Added a new enum `ToggleState`.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1088#discussion_r1192205065
PR Review Comment: https://git.openjdk.org/jfx/pull/1088#discussion_r1192204855

Reply via email to