Skip to content

Fix high contrast issues for palette editor and extensions page#9434

Merged
kimprice merged 1 commit into
masterfrom
kim-fix-hc
Mar 22, 2023
Merged

Fix high contrast issues for palette editor and extensions page#9434
kimprice merged 1 commit into
masterfrom
kim-fix-hc

Conversation

@kimprice
Copy link
Copy Markdown
Contributor

Fixes microsoft/pxt-arcade#5701
Fixes microsoft/pxt-microbit#4997

At first glance, high contrast seemed to be working, but that was only the case if you had just turned high contrast on. If instead, you already had high contrast on from a previous session or reloaded the page, the palette editor and extensions page would not be in high contrast mode.

Copy link
Copy Markdown
Member

@jwunderl jwunderl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

}
</div>
</FocusTrap>, parentElement || document.body)
</FocusTrap>, parentElement || document.getElementById("root") || document.body)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to clarify a bit in case anyone's confused what this actually changes; parentElement is an optional prop that determines where this this modal should be rooted (as modals are things that should 'show up' above other things and so they need to get positioned at a different place in the dom). When we were looking at it yesterday we noticed that not all modals were passing a value for this and were instead falling back to being placed under the body instead of root where .hc gets applied (#root is the ProjectView's id in app.tsx).

If we're in the webapp almost everything is intended to be under root anyway which is why I suggested this vs going back and making parentElement required~

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for including the reasoning, Joey! I should have added those details.

@kimprice kimprice merged commit 0b7c867 into master Mar 22, 2023
@kimprice kimprice deleted the kim-fix-hc branch March 22, 2023 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extensions page does not support high contrast mode Color palette: high contrast not working

3 participants