fix: [M3-8453] - Update error text color in confirmation dialogs to use theme, refactor styling #10828
Conversation
| @@ -2,9 +2,12 @@ import { styled } from '@mui/material/styles'; | |||
| import * as React from 'react'; | |||
| import { useStyles } from 'tss-react/mui'; | |||
|
|
|||
| import { Button, ButtonProps } from 'src/components/Button/Button'; | |||
There was a problem hiding this comment.
this and Dialog.tsx are just eslint changes I saw when I landed on those files while working on this ticket
…hardcoded to using the theme color
|
Is 'Changed' the right category for this changeset, or would it be 'Fixed'? |
| const StyledErrorText = styled(DialogContentText, { | ||
| label: 'StyledErrorText', | ||
| })(({ theme }) => ({ | ||
| color: theme.palette.error.dark, |
There was a problem hiding this comment.
...the actual assigned part of this ticket lol
thanks @jaalah-akamai for letting me know which theme color to use!
There was a problem hiding this comment.
🚀 Thanks, Connie! Looking much better in dark mode now and you did some good clean up, too.
Is 'Changed' the right category for this changeset, or would it be 'Fixed'?
Since it was an incorrect color and (more importantly) that low contrast was an accessibility issue, I would lean towards Fixed and rewording the changeset to be something like: Inaccessible, non-theme error text color in confirmation dialogs. What you have does work, though.
Should I add another changeset for the unit tests and/or style refactoring too?
I wouldn't add a changeset for style refactoring. Adding a Test changeset for the new unit test is up to you - and we could probably be more consistent about when to do this as a team. I think it's a good idea because it's a new file to test a component we didn't have coverage for previously, as opposed to a new test case for an existing file, which I usually don't bother adding a separate changeset for. Thanks for the test coverage!
|
Coverage Report: ✅ |
Description 📝
Update error text color in confirmation dialogs to not be hardcoded, as per discussion. Also added some tests/refactored styling to use styled components while I was at it
Changes 🔄
Target release date 🗓️
n/a
Preview
How to test 🧪
Verification steps
As an Author I have considered 🤔
Check all that apply