fix(help): accessible colors for help text statuses

[cooper-joe]

This PR changes the color of warning and valid variations of the help component. The previous colors were not AA accessible.

I have replaced theme.valid and theme.warning with the color variables because the theme values are used elsewhere where the contrast requirements are not as strict. I contemplated adding theme.warning-dark and theme.valid-dark, but I thought that was over-engineering this. I am open to feedback if that would indeed be a better solution. Perhaps theme.warning-text might be a useful addition?

[cypress]

---

Test summary

502 • 0 • 0 • 0

---

Run details

Project	ui
Status	Passed
Commit	fd63bc0
Started	Mar 5, 2021 3:15 PM
Ended	Mar 5, 2021 3:21 PM
Duration	05:15 💡
OS	Linux Ubuntu - 20.04
Browser	Electron 87

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[ghost]

I have replaced theme.valid and theme.warning with the color variables because the theme values are used elsewhere where the contrast requirements are not as strict.

Would it be harmful if we just update the theme values to the colors with the proper contrast? It might not strictly be necessary everywhere, but if it's not harmful that seems like the easiest solution to me. What do you think @cooper-joe?

[cooper-joe]

I have replaced theme.valid and theme.warning with the color variables because the theme values are used elsewhere where the contrast requirements are not as strict.

Would it be harmful if we just update the theme values to the colors with the proper contrast? It might not strictly be necessary everywhere, but if it's not harmful that seems like the easiest solution to me. What do you think @cooper-joe?

This is certainly simpler yes, though I think that this might causes headaches for people using those warning or valid variations in other places - for example as fills or backgrounds. Without knowing all of those use cases I feel like changing the value here would not be good.

I am leaning towards having two theme values: warning and warning-dark or warning-text-accessible. There are still instances where warning is useful. Having a single warning color is not ideal, just like having a single yellow or red wouldn't work. Perhaps this is an argument for a warning900 – warning050 system?

[ghost]

I have replaced theme.valid and theme.warning with the color variables because the theme values are used elsewhere where the contrast requirements are not as strict.

Would it be harmful if we just update the theme values to the colors with the proper contrast? It might not strictly be necessary everywhere, but if it's not harmful that seems like the easiest solution to me. What do you think @cooper-joe?

This is certainly simpler yes, though I think that this might causes headaches for people using those warning or valid variations in other places - for example as fills or backgrounds. Without knowing all of those use cases I feel like changing the value here would not be good.

I am leaning towards having two theme values: warning and warning-dark or warning-text-accessible. There are still instances where warning is useful. Having a single warning color is not ideal, just like having a single yellow or red wouldn't work. Perhaps this is an argument for a warning900 – warning050 system?

Yeah tricky. On the one hand ensuring good contrast without the developers having to pay too much attention to it would be nice. On the other hand, it's difficult for us to predict all usecases.

warning900 – warning050 system?

I can see the use of that, but then again, at that point they would just be aliases of yellow900 to yellow050, etc, right? I think I'd be fine with just having the colors (yellow, blue, red, etc.). It's well established what colors we use where, so dropping the warning, error, info colors seems likea good solution to me 👍

[ghost]

Oh wait. I see now that you didn't want to drop them entirely. In that case it does introduce some inconsistencies. Like as a developer, would it be obvious to you why we're using the error color for the error, but the yellow and blue for the other states? Do we add comments everywhere where we're not using the error, warning, info color aliases that this is for contrast? I'm fine with this particular change, but maybe we should think about a nice generic way to approach this elsewhere? Maybe the design repo?

[ghost]

I'm ok with these changes 👍. @dhis2/team-apps anyone have any strong feelings against?

[ghost]

@cooper-joe This is fine to merge I think. Unless there's something that still needs to be done that I'm not aware of?

[dhis2-bot]

🎉 This PR is included in version 6.5.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀