Accessibility compliance: Improve share dialog accessibility #9989
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…erk/share-dialog-accessibility
…erk/share-dialog-accessibility
riknoll
reviewed
Apr 29, 2024
riknoll
requested changes
Apr 29, 2024
This reverts commit 22df435.
…nsize breakpoint, modified embed css change
…e in focustrap, unite it with focusable for focuslist
…erk/share-dialog-accessibility
riknoll
reviewed
May 8, 2024
riknoll
reviewed
May 8, 2024
react-common/components/util.tsx
Outdated
| @@ -91,4 +91,25 @@ export function screenToSVGCoord(ref: SVGSVGElement, coord: ClientCoordinates) { | |||
| screenCoord.x = coord.clientX; | |||
| screenCoord.y = coord.clientY; | |||
| return screenCoord.matrixTransform(ref.getScreenCTM().inverse()); | |||
| } | |||
|
|
|||
| export function findNextFocusableElement(elements: HTMLElement[], focusedIndex: number, index: number, forward: boolean): HTMLElement { | |||
There was a problem hiding this comment.
nit: you can eliminate the focusedIndex argument by using the iterative implementation above instead of the recursive one
There was a problem hiding this comment.
I prefer using recursion for this, but would like to hear more of your thoughts if you feel strongly about implementing it with iteration.
There was a problem hiding this comment.
the iterative implementation doesn't require passing in focusedIndex. I just also generally prefer loops to tail recursion, personally (but feel free to ignore)
riknoll
reviewed
May 8, 2024
riknoll
reviewed
May 8, 2024
…neric for simulator thumbnail, got rid of important on embed button styling
riknoll
approved these changes
May 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've made a couple of changes with this PR, but I've done so much fiddling and changing and testing across these commits that it's hard all of what's changed. The code in this PR does fix the following issues:
Fixes microsoft/pxt-microbit#5468
Fixes microsoft/pxt-microbit#5472
Fixes microsoft/pxt-microbit#5488
Fixes microsoft/pxt-microbit#5471
I also did some work for this issue . I finagled with this a bit and got it to a place that looked okay, but it really made the other zoom and screen experiences just bad, so I scratched the progress made. I did make a change, though, that if a screen is small enough, I change the height of the modal overlay to be 95% of the screen so more of the modal is usable.^ this no longer applies as making the modal scrollable fixes the problem
It will be easier to see these changes just by playing with difference zooms, screen sizes, and resolutions. Let me know if there are other things that I should adjust.
Upload target: https://makecode.microbit.org/app/44c2bdbef7436f5a3215b42b34cf672cb13298ac-2ae3b742d0#editorNew upload target: https://makecode.microbit.org/app/9cc334c6852c269cc944ea1241973ece3d4a12a6-950d2c09e5#