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