On Tue, 9 May 2023 15:08:30 GMT, Kevin Rushforth <[email protected]> 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
