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

fix(material/select): arrow not rendering correctly in high contrast mode #14219

Closed
wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

Since all borders get rendered out in high contrast mode, we can't use a CSS triangle to render out the mat-select dropdown arrow. These changes replace it with an SVG arrow.

Fixes #14207.

@crisbeto crisbeto added Accessibility This issue is related to accessibility (a11y) target: patch This PR is targeted for the next patch release labels Nov 20, 2018
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 20, 2018
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@josephperrott josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Nov 27, 2018
@jelbourn
Copy link
Member

@crisbeto this causes the arrow to be in a slightly different position, causing a very large number of screenshot diffs. It would be way easier to merge if it was in exactly the same position.

@crisbeto
Copy link
Member Author

crisbeto commented Nov 28, 2018

@jelbourn do we know what browsers it happens on? I was doing a manual screenshot diff as I was doing it and it looked identical.

@jelbourn
Copy link
Member

All the screenshot tests in Google are generally just taken with Chrome

@mmalerba mmalerba added aaa and removed aaa labels Apr 25, 2019
@andrewseguin andrewseguin added the P4 A relatively minor issue that is not relevant to core functions label May 30, 2019
@crisbeto crisbeto force-pushed the 14207/select-svg-arrow branch from 78d766e to fa48769 Compare June 20, 2019 21:07
@josephperrott josephperrott removed their assignment Oct 15, 2019
@mmalerba mmalerba removed the lgtm label Jul 31, 2020
@jelbourn
Copy link
Member

@crisbeto should we revisit this PR? I just checked this for the existing and MDC-based MatSelect and both render a little rectangle in place of the dropdown arrow.

@crisbeto
Copy link
Member Author

Sorry for the delay @jelbourn, I somehow missed your previous message and I saw it now after revisiting this PR. I think that we should try it again, because we've had more reports about this being an issue. Should we trigger another presubmit to see if it's still a problem? I just tried it out again, and it still looks identical.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed P4 A relatively minor issue that is not relevant to core functions labels Jan 15, 2021
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jan 15, 2021
…ntrast mode

The MDC select uses a CSS triangle for its down arrow which renders as a rectangle in
high contrast mode. These changes switch it to an SVG arrow.

These changes are identical to angular#14219 which had some trouble landing due to screenshot
diffs. The MDC select shouldn't have such issues.

Relates to angular#21263.
@crisbeto crisbeto changed the title fix(select): arrow not rendering correctly in high contrast mode fix(material/select): arrow not rendering correctly in high contrast mode Jan 15, 2021
@crisbeto crisbeto force-pushed the 14207/select-svg-arrow branch 2 times, most recently from ed0a833 to 4144b2e Compare January 19, 2021 21:37
…mode

Since all borders get rendered out in high contrast mode, we can't use a CSS triangle to
render out the `mat-select` dropdown arrow. These changes replace it with an SVG arrow.

Fixes angular#14207.
@crisbeto crisbeto force-pushed the 14207/select-svg-arrow branch from 4144b2e to 7609ba1 Compare January 19, 2021 21:47
@jelbourn
Copy link
Member

sgtm, @andrewseguin we should get a fresh presubmit run for this

annieyw pushed a commit that referenced this pull request Mar 7, 2021
…ntrast mode (#21606)

The MDC select uses a CSS triangle for its down arrow which renders as a rectangle in
high contrast mode. These changes switch it to an SVG arrow.

These changes are identical to #14219 which had some trouble landing due to screenshot
diffs. The MDC select shouldn't have such issues.

Relates to #21263.
annieyw pushed a commit that referenced this pull request Mar 7, 2021
…ntrast mode (#21606)

The MDC select uses a CSS triangle for its down arrow which renders as a rectangle in
high contrast mode. These changes switch it to an SVG arrow.

These changes are identical to #14219 which had some trouble landing due to screenshot
diffs. The MDC select shouldn't have such issues.

Relates to #21263.

(cherry picked from commit fef9179)
@andrewseguin andrewseguin added needs rebase and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Dec 29, 2021
@crisbeto
Copy link
Member Author

Closing, because this causes a large number of screenshot diffs internally and the issue has been fixed in the MDC version of the select.

@crisbeto crisbeto closed this Feb 26, 2022
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MatSelect: dropdown arrow renders as rectangle in high-contrast mode
6 participants