-
Notifications
You must be signed in to change notification settings - Fork 0
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
SCC-4389: My Account 2.0 QA round 1 and VQA/a11y round 2 #418
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
> | ||
{selection} | ||
</Text> | ||
{type === "notification" && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like this same check is used twice. can you pull it out into a variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now isNotification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Vera confirmed offline that it's good to go so I am dismissing her review |
Confirmed offline that it's good to go
Ticket:
This PR does the following:
Resolves the following issues:
a11y:
QA:
** New product requirements, introduced 12/12
How has this been tested?
Most of these updates can only be seen on an account that has no attached phone number or email, and has its notification preference set to None. Use this one: 25555001351471 / 1082 but DO NOT actually Save Changes on an email/phone or on the notification preference, so that you can both see what's going on :)
Accessibility concerns or updates
Checklist: