fix(a11y): mgt-file and mgt-picker visibility issues in dark-mode #2667
Conversation
|
Thank you for creating a Pull Request @Mnickii. This is a checklist for the PR reviewer(s) to complete before approving and merging this PR:
|
|
The updated storybook is available here |
|
The updated storybook is available here |
|
Hi @gavinbarron, This issue is still reproduced as of now proper text is still not properly visible when user activates the 'Color Mode' toggle button. Also, we have checked the Color Contrast for the text, and it will be less than 4.5:1 due to which this issue still appears and same for the placeholder field under the 'Picker' so please refer to the snippet below for the reference. Snippet:
|
|
@Mnickii for the picker you're likely need to use the |
|
The updated storybook is available here |
|
The updated storybook is available here |
|
The updated storybook is available here |
Co-authored-by: Musale Martin <martinmusale@microsoft.com>
|
The updated storybook is available here |
|
@gavinbarron in the last meeting, @yejuntak mentioned that there will be a figma update on dark mode colors, so that's what we're waiting on for this. |
|
Ping @yejuntak. Where are we with the color updates? |
|
@sebastienlevert I have shared my progress with the team at the last sync. I am currently implementing the whole auto-recoloring (web variables and mode) new features in Figma, to resolve all the similar issues permanently. However, since our Figma File was using too many unnecessary multiple color sources, changing the color to web variables are longer than what I thought. As soon as I finish test variables, I will prioritize MGT file and MGT picker then rest of the components. |
|
This PR is sitting here because of a Figma update? Can you share the color codes for now so we can expedite this change to the next version? Figma updates can happen after. Thanks. @yejuntak |
|
The updated storybook is available here |
|
The updated storybook is available here |
1 similar comment
|
The updated storybook is available here |
|
@yejuntak yes, 4.5:1 is the required contrast ratio for regular text: |
|
@Mnickii great work so far here. I know this one has been a lot more work than it seemed at first glance. Although there is a link between the color work here and the changes to the people-picker I would have preferred a separate PR for that change. What's done is done now, but in future the more we can keep work separated the better. I think that the placeholder text color for our mgt-picker is still not right. It looks like it uses the same color as the input and selected item text, and it really should not. @yejuntak can you please let us know what colors the placeholder text needs to be? |
|
@gavinbarron Currently all the input placeholder texts are foreground 4 #707070. I have referenced from chat component. It is probably one of those colors where we have the Fluent design langage have the color but somehow we don't have it. @Mnickii Let me know if there is any other issues |
|
The updated storybook is available here |
|
The updated storybook is available here |
|
The updated storybook is available here |
|
The updated storybook is available here |
Closes #2318
Closes #2727
PR Type
Bugfix
Description of the changes
Fixes file line2 and 3 visibilities in dark mode
Fixes picker placeholder visibility in dark mode
PR checklist
yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)yarn setLicense)Other information