Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop. Don't use AccessibleRole.PANEL for root/unknown nodes (fixes issues on Windows) #885

Merged
merged 4 commits into from Oct 20, 2023

Conversation

igordmn
Copy link
Collaborator

@igordmn igordmn commented Oct 19, 2023

This fixes issues on Windows:

  1. Windows pronounced "Panel unavailable" at start
  2. When we focus on Switch or Box(Modifier.clickable), there was "Panel0" pronounced. Now it says nothing.
  3. After merging Desktop. Fix input methods on JBR, disable input methods when we lose focus #881, we refocus the main component. This leads to pronouncing the window title, now it says nothing, as the accessible root node is unknown

on macOs it changes one thing:

  1. When we focus on Switch or Box(Modifier.clickable), now it prounonces that we in scroll area (if we in scroll area). It also pronounces it for other compnents like Button/Text

Testing

Manually with accessibility enabled in run1 (Windows, macOs)

…ixes issues on Windows)

This fixes issues on Windows:

1. Windows pronounced "Panel unavailable" at start
2. When we focus on Switch or Box(Modifier.clickable), there was "Panel0" pronounced. Now it says nothing.
3. After merging #881 refocus the main component. This leads to pronouncing the window title, now it says nothing, as the accessible root node is unknown

on macOs it doesn't change anything

## Testing
Manually with accessibility enabled on run1 (Windows, macOs)
@igordmn igordmn changed the title Remove Panel accessible role from the root node, and unknown nodes Don't use AccessibleRole.PANEL for root/unknown nodes (fixes issues on Windows) Oct 20, 2023
@igordmn igordmn changed the title Don't use AccessibleRole.PANEL for root/unknown nodes (fixes issues on Windows) Desktop. Don't use AccessibleRole.PANEL for root/unknown nodes (fixes issues on Windows) Oct 20, 2023
@igordmn igordmn marked this pull request as ready for review October 20, 2023 01:12
@igordmn igordmn requested a review from m-sasha October 20, 2023 01:12
@m-sasha
Copy link

m-sasha commented Oct 20, 2023

To be honest, I don't know whether this is correct (or what is the correct thing to do), and whether it will break something. However, I suggest we adopt a strategy of not being afraid to break things with a11y, so that we learn quickly and be able to move forward.

On the one hand, AccessibleRole.Panel seems to be the right thing to return for a generic container like Box. On the other hand, Box/Column/Row aren't really used as containers, but as layout tools. Looking at the Compose semantic properties, however, maybe we should assign AccessibleRole.Panel to SemanticsPropertyReceiver.isContainer and/or SemanticsPropertyReceiver.isTraversalGroup.

@igordmn
Copy link
Collaborator Author

igordmn commented Oct 20, 2023

I added "isTraversalGroup != null -> AccessibleRole.GROUP_BOX". I works only if users explicitly set isTraversalGroup = true

macOs pronounces "Group", Windows "Grouping".

With Panel, macOs pronounces nothing, Windows - "Panel".

@igordmn igordmn merged commit 8748d3e into jb-main Oct 20, 2023
2 of 3 checks passed
@igordmn igordmn deleted the igor.demin/fix-panel-role branch October 20, 2023 18:38
igordmn added a commit that referenced this pull request Oct 20, 2023
… focus (#881)

### Rerequest focus on main component when we need to type using input
methods
Fixes JetBrains/compose-multiplatform#2628

The issue was because of 2 things:
- we used a hack to force a focused event
(`component.inputContext.dispatchEvent(focusGainedEvent)`
- JBR added optimization to ignore focus on the same element (thanks
@AiHMin for investigation
[here](JetBrains/compose-multiplatform#2628 (comment))).
In Compose we have only one element.
Because optimization is correct, and the hack depends on the internals,
it isn't right to fix it in JBR, we should fix it in Compose.
Furthermore, even without JBR changes, this hack didn't complete work -
we can't for example use it for disabling input methods (see the next
point).

In this PR we also use a hack unfortenutely - we refocus the root
component, focusing on invisible component first. That leads to another
issue with acccessibility, but we fix it
[here](#885)).

A proper fix should be switching to native code, or making an API in JBR
(but other vendors still be unsupported).

### Don't show input methods popup if there is no focused TextField 

Previously we showed a popup, even if there are no focused textfield:

![image](https://github.com/JetBrains/compose-multiplatform-core/assets/5963351/82e3543f-11f4-4013-8da5-d782b824ed2c)
Now we don't show it if we isn't in a textfield.
P.S. only on Windows/Linux for now, on macOs Swing seems has [a
bug](JetBrains/compose-multiplatform#3839)

## Testing
Tested manually on:
1. Windows, Chinese/Korean/Japanese, OpenJDK/JBR 17, Accessibility
enabled/disabled, ComposeWindow/ComposePanel
2. macOs, Chinese, OpenJDK/JBR 17, Accessibility enabled/disabled
4. Linux, Chinese layout, OpenJDK 17
igordmn added a commit that referenced this pull request Oct 20, 2023
… issues on Windows) (#885)

This fixes issues on Windows:

1. Windows pronounced "Panel unavailable" at start
2. When we focus on Switch or Box(Modifier.clickable), there was
"Panel0" pronounced. Now it says nothing.
3. After merging
#881, we
refocus the main component. This leads to pronouncing the window title,
now it says nothing, as the accessible root node is unknown

on macOs it changes one thing:
1. When we focus on Switch or Box(Modifier.clickable), now it prounonces
that we in scroll area (if we in scroll area). It also pronounces it for
other compnents like Button/Text

## Testing
Manually with accessibility enabled in run1 (Windows, macOs)
igordmn added a commit that referenced this pull request Oct 20, 2023
… focus (#881)

### Rerequest focus on main component when we need to type using input
methods
Fixes JetBrains/compose-multiplatform#2628

The issue was because of 2 things:
- we used a hack to force a focused event
(`component.inputContext.dispatchEvent(focusGainedEvent)`
- JBR added optimization to ignore focus on the same element (thanks
@AiHMin for investigation
[here](JetBrains/compose-multiplatform#2628 (comment))).
In Compose we have only one element.
Because optimization is correct, and the hack depends on the internals,
it isn't right to fix it in JBR, we should fix it in Compose.
Furthermore, even without JBR changes, this hack didn't complete work -
we can't for example use it for disabling input methods (see the next
point).

In this PR we also use a hack unfortenutely - we refocus the root
component, focusing on invisible component first. That leads to another
issue with acccessibility, but we fix it
[here](#885)).

A proper fix should be switching to native code, or making an API in JBR
(but other vendors still be unsupported).

### Don't show input methods popup if there is no focused TextField 

Previously we showed a popup, even if there are no focused textfield:

![image](https://github.com/JetBrains/compose-multiplatform-core/assets/5963351/82e3543f-11f4-4013-8da5-d782b824ed2c)
Now we don't show it if we isn't in a textfield.
P.S. only on Windows/Linux for now, on macOs Swing seems has [a
bug](JetBrains/compose-multiplatform#3839)

## Testing
Tested manually on:
1. Windows, Chinese/Korean/Japanese, OpenJDK/JBR 17, Accessibility
enabled/disabled, ComposeWindow/ComposePanel
2. macOs, Chinese, OpenJDK/JBR 17, Accessibility enabled/disabled
4. Linux, Chinese layout, OpenJDK 17
mazunin-v-jb pushed a commit that referenced this pull request Dec 7, 2023
… issues on Windows) (#885)

This fixes issues on Windows:

1. Windows pronounced "Panel unavailable" at start
2. When we focus on Switch or Box(Modifier.clickable), there was
"Panel0" pronounced. Now it says nothing.
3. After merging
#881, we
refocus the main component. This leads to pronouncing the window title,
now it says nothing, as the accessible root node is unknown

on macOs it changes one thing:
1. When we focus on Switch or Box(Modifier.clickable), now it prounonces
that we in scroll area (if we in scroll area). It also pronounces it for
other compnents like Button/Text

## Testing
Manually with accessibility enabled in run1 (Windows, macOs)
mazunin-v-jb pushed a commit that referenced this pull request Dec 7, 2023
… focus (#881)

### Rerequest focus on main component when we need to type using input
methods
Fixes JetBrains/compose-multiplatform#2628

The issue was because of 2 things:
- we used a hack to force a focused event
(`component.inputContext.dispatchEvent(focusGainedEvent)`
- JBR added optimization to ignore focus on the same element (thanks
@AiHMin for investigation
[here](JetBrains/compose-multiplatform#2628 (comment))).
In Compose we have only one element.
Because optimization is correct, and the hack depends on the internals,
it isn't right to fix it in JBR, we should fix it in Compose.
Furthermore, even without JBR changes, this hack didn't complete work -
we can't for example use it for disabling input methods (see the next
point).

In this PR we also use a hack unfortenutely - we refocus the root
component, focusing on invisible component first. That leads to another
issue with acccessibility, but we fix it
[here](#885)).

A proper fix should be switching to native code, or making an API in JBR
(but other vendors still be unsupported).

### Don't show input methods popup if there is no focused TextField 

Previously we showed a popup, even if there are no focused textfield:

![image](https://github.com/JetBrains/compose-multiplatform-core/assets/5963351/82e3543f-11f4-4013-8da5-d782b824ed2c)
Now we don't show it if we isn't in a textfield.
P.S. only on Windows/Linux for now, on macOs Swing seems has [a
bug](JetBrains/compose-multiplatform#3839)

## Testing
Tested manually on:
1. Windows, Chinese/Korean/Japanese, OpenJDK/JBR 17, Accessibility
enabled/disabled, ComposeWindow/ComposePanel
2. macOs, Chinese, OpenJDK/JBR 17, Accessibility enabled/disabled
4. Linux, Chinese layout, OpenJDK 17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants