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: presence icons for dark forced colors #2817

Merged
merged 3 commits into from
Nov 10, 2023

Conversation

gavinbarron
Copy link
Member

Closes #2799

PR Type

  • Bugfix
  • Accessibility

Description of the changes

fixes fill color for svgs in presence badges when using forced colors preferring dark

PR checklist

  • Project builds (yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)
  • All public APIs (classes, methods, etc) have been documented following the jsdoc syntax
  • Stories have been added and existing stories have been tested
  • Added appropriate documentation. Docs PR:
  • License header has been added to all new source files (yarn setLicense)
  • Contains NO breaking changes

Other information

@gavinbarron
Copy link
Member Author

@vagpt I believe this change fixes the presence badge issue in high contrast modes

Copy link

📖 The updated storybook is available here

1 similar comment
Copy link

📖 The updated storybook is available here

Copy link
Collaborator

@Mnickii Mnickii left a comment

Choose a reason for hiding this comment

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

LGTM! Works as expected

Copy link

github-actions bot commented Nov 2, 2023

📖 The updated storybook is available here

@gavinbarron
Copy link
Member Author

@vagpt could you please review this change?

I'd like to get it merged soon.

@vagpt
Copy link
Collaborator

vagpt commented Nov 2, 2023

Hi @gavinbarron,

This issue is partially fixed as of now this issue working fine for the 'Desert' mode only. For the rest theme i.e. 'Aquatic, Dusk and Night Sky' issue is still repro.

Please refer to the snippet below for the reference.

Test Environment:
URL: https://mgt.dev/next/pr/2817/?path=/story/components-mgt-person-properties--person-presence-display-all
OS Version: 23H2 (OS Build 25977.1010)
Browser Version: Edge Dev 120.0.2186.2

Aquatic Theme:
image

Desert Theme:
image

@gavinbarron
Copy link
Member Author

@vagpt, the icons are clearly different for each case now, it just happens that they are using black and white as would be expected in a high contrast theme.

Please advise on the expected behavior for our dark high contrast themes?

@vagpt
Copy link
Collaborator

vagpt commented Nov 3, 2023

@vagpt, the icons are clearly different for each case now, it just happens that they are using black and white as would be expected in a high contrast theme.

Please advise on the expected behavior for our dark high contrast themes?

Hi @gavinbarron, Agreed with your comment above.

We have checked the Dark Mode as well and there all the icons are properly visible. Also, in system high contrast theme there is no guideline is provided for the symbols please refer to the snippet below for reference.

image

Also, we have few suggestions for the icons appeared in Aquatic Theme i.e. it would be great if the tooltip will be appear on the every icons so that in high contrast theme user can easily identify about the status.
Also, it would be great if the same UI will be appear for the icons in high contrast Aquatic theme as it is visible on the Dark mode.

Please let me know if you have any query.

@gavinbarron
Copy link
Member Author

Thanks @vagpt.
We do provide a tooltip with the text for the status on all of the badges in all color modes:
presence badge showing a tooltip in aquatic high contrast mode

When a user set the accessibility contrast theme to Aquatic, Dusk, or Night Sky we identify all of these in the browser the same way, @media (forced-colors: active) and (prefers-color-scheme: dark)
Conversely, we use @media (forced-colors: active) and (prefers-color-scheme: light) as a means of detecting the Desert theme.

If I'm understanding your guidance correctly then we should use the same icon colors in forced color mode and regular mode? That would look like this:
presence icons with color in Aquatic contrast theme

Personally, I think that only using white and black results in an easier to distinguish result:
white and dark presence icons

If this would benefit from an interactive session to help land on the right solution and talk it through then please set up a meeting so that we can resolve this issue as quickly as possible.

@gavinbarron
Copy link
Member Author

@vagpt this is awaiting your feedback

@vagpt
Copy link
Collaborator

vagpt commented Nov 9, 2023

@vagpt this is awaiting your feedback

Hi @gavinbarron

Sorry for the delayed in response.

If we are getting tooltip for the badges in Aquatic and other mode then we are good.

@gavinbarron
Copy link
Member Author

Thanks @vagpt, given this feedback, I'm going to merge this change and you'll add the relevant tags for resolution.

Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
mgt-components.src.components 81% 100% 0
mgt-components.src.components.mgt-contact 68% 83% 0
mgt-components.src.components.mgt-file 51% 100% 0
mgt-components.src.components.mgt-file-list 56% 100% 0
mgt-components.src.components.mgt-file-list.mgt-file-upload 49% 88% 0
mgt-components.src.components.mgt-messages 66% 100% 0
mgt-components.src.components.mgt-organization 47% 100% 0
mgt-components.src.components.mgt-person 79% 65% 0
mgt-components.src.components.mgt-person-card 64% 39% 0
mgt-components.src.components.mgt-profile 40% 100% 0
mgt-components.src.components.mgt-theme-toggle 100% 100% 0
mgt-components.src.components.sub-components.mgt-flyout 72% 53% 0
mgt-components.src.components.sub-components.mgt-spinner 100% 100% 0
mgt-components.src.graph 38% 89% 0
mgt-components.src.styles 92% 80% 0
mgt-components.src.utils 79% 28% 0
mgt-element.dist.es6.components.src.components 73% 79% 0
mgt-element.dist.es6.mock.src.mock 90% 72% 0
mgt-element.dist.es6.providers.src.providers 85% 69% 0
mgt-element.dist.es6.src 91% 80% 0
mgt-element.dist.es6.utils.src.utils 66% 69% 0
mgt-element.src 88% 100% 0
mgt-element.src.components 84% 100% 0
mgt-element.src.mock 81% 56% 0
mgt-element.src.providers 80% 85% 0
mgt-element.src.utils 71% 90% 0
Summary 65% (12450 / 19095) 64% (407 / 640) 0

Copy link

📖 The updated storybook is available here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
5 participants