Skip to content

Luminosity contrast accessibility bugs (568, 569, 589) #2693#2700

Merged
mhdahman merged 4 commits intomasterfrom
mhdahman/a11y-luminosity-contrast-2
Apr 6, 2023
Merged

Luminosity contrast accessibility bugs (568, 569, 589) #2693#2700
mhdahman merged 4 commits intomasterfrom
mhdahman/a11y-luminosity-contrast-2

Conversation

@mhdahman
Copy link
Contributor

@mhdahman mhdahman commented Mar 31, 2023 •
edited
Loading

Resolves:

  • [Bug 568] [Visual Requirement-Batch Explorer-Add a batch account] Luminosity contrast ratio of the text "eastus.batch.azure.com" is less than 4.5:1 i.e 2.5:1.
  • [Bug 569] [Visual Requirement-Batch Explorer-Node states] Luminosity contrast ratio of the text "Idle" is less than 4.5:1 i.e 2.5:1.
  • [Bug 589] [Visual Requirement-Batch Explorer-Add a pool to the account] On the Add a pool to the account blade, luminosity contrast ratio of the text " Container" is less than 4.5:1 i.e 3.7:1.

Notable changes:

  • Updated colors of monitor chart and other parts of the UI on the dashboard for both classic and dark themes to have luminosity contrast ratio >=4.5:1 — bug above was specific to classic theme
  • Update primary text color to a slightly darker color to pass the contrast test. This should affect all places using the $primary-color.
  • Updated .mat-tab-label and .mat-tab-label-active to pass the contrast test. This should affect all instances of the material ui tables.
  • Updated styles of bl-form-field to have luminosity contrast ratio >=4.5:1. This change affects all instances of <bl-form-field> that use blFormFieldSuffix or blFormFieldPrefix.
  • Update tooltip configuration of the "Node states" chart to improve visibility. Please see my inline comment below for additional information.

Dashboard – classic theme:

Screen Shot 2023-03-23 at 1 32 56 PM

Dashboard – dark theme:

Screen Shot 2023-03-23 at 1 32 17 PM

Form fields with suffix – classic theme:

Screen Shot 2023-03-23 at 1 51 15 PM

Form fields with suffix – dark theme:

Screen Shot 2023-03-23 at 2 07 06 PM

mhdahman
mhdahman commented Mar 31, 2023
View reviewed changes
src/app/components/account/monitoring/monitor-chart/monitor-chart.component.ts
Comment on lines -199 to +200
mode: "index",
mode: "single",
position: "nearest",
Copy link
Contributor Author

@mhdahman mhdahman Mar 31, 2023

Choose a reason for hiding this comment

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

Update the tooltip configuration of monitor chart to improve the visibility of the tooltip. Please note that this changes the existing behavior of the tooltip to only show information about the hovered datapoint rather than displaying all datapoints at the hovered index which wasn't helpful because all datapoints could not be displayed within a limited space.

Before

Screen Shot 2023-03-23 at 2 19 08 PM

Screen Shot 2023-03-23 at 2 19 03 PM

Screen Shot 2023-03-23 at 2 18 58 PM

After

Screen Shot 2023-03-23 at 10 41 16 AM

Screen Shot 2023-03-23 at 10 40 50 AM

Screen Shot 2023-03-23 at 10 41 20 AM

@mhdahman mhdahman mentioned this pull request Mar 31, 2023
Closed
gingi
gingi previously approved these changes Mar 31, 2023
View reviewed changes
@mhdahman mhdahman force-pushed the mhdahman/a11y-luminosity-contrast-2 branch from 0fc8c63 to b254ef2 Compare March 31, 2023 18:21
@codecov
Copy link

codecov bot commented Mar 31, 2023 •
edited
Loading

Codecov Report

Merging #2700 (3a84777) into master (d1abd51) will not change coverage.
The diff coverage is n/a.

❗ Current head 3a84777 differs from pull request most recent head c7bb9bb. Consider uploading reports for the commit c7bb9bb to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2700   +/-   ##
=======================================
  Coverage   65.09%   65.09%           
=======================================
  Files         916      916           
  Lines       26475    26475           
  Branches     5237     5237           
=======================================
  Hits        17233    17233           
  Misses       9242     9242
Impacted Files Coverage Δ
...onitoring/monitor-chart/monitor-chart.component.ts 78.57% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1abd51...c7bb9bb. Read the comment docs.

@mhdahman mhdahman changed the title Luminosity contrast accessibility bugs (568, 569) #2693 Luminosity contrast accessibility bugs (568, 569, 589) #2693 Mar 31, 2023
cRui861
cRui861 previously approved these changes Apr 4, 2023
View reviewed changes
Copy link
Member

@cRui861 cRui861 left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes! Looks good :)

@mhdahman mhdahman force-pushed the mhdahman/a11y-luminosity-contrast-2 branch from b254ef2 to a1f577c Compare April 5, 2023 20:31
@mhdahman mhdahman requested a review from cRui861 April 5, 2023 21:49
@mhdahman
Copy link
Contributor Author

mhdahman commented Apr 5, 2023

Updated branch with master, can I please get another approval?

@mhdahman mhdahman requested a review from gingi April 6, 2023 19:00
@mhdahman mhdahman force-pushed the mhdahman/a11y-luminosity-contrast-2 branch from a1f577c to c7bb9bb Compare April 6, 2023 20:13
@mhdahman mhdahman merged commit cbd0b1d into master Apr 6, 2023
@mhdahman mhdahman deleted the mhdahman/a11y-luminosity-contrast-2 branch April 6, 2023 20:50
@cRui861
Copy link
Member

cRui861 commented Apr 26, 2023

Fixes #2713

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gingi gingi gingi approved these changes

@dpwatrous dpwatrous Awaiting requested review from dpwatrous dpwatrous is a code owner

@zfengms zfengms Awaiting requested review from zfengms zfengms is a code owner

@paterasMSFT paterasMSFT Awaiting requested review from paterasMSFT

@seth-barshay seth-barshay Awaiting requested review from seth-barshay

@wiboris wiboris Awaiting requested review from wiboris wiboris is a code owner

@skapur12 skapur12 Awaiting requested review from skapur12 skapur12 is a code owner

@NickKouds NickKouds Awaiting requested review from NickKouds NickKouds is a code owner

@cRui861 cRui861 Awaiting requested review from cRui861 cRui861 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mhdahman @cRui861 @gingi