-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add SecurityWarningModal to the layout page #2044
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
af3e0a8
to
798da10
Compare
798da10
to
4d26c9d
Compare
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.
Thanks for the quick implementation, Oleg
Looks great, just a couple of nits in the comments 🔥
const [openedAccordionItems, setOpenedAccordionItems] = useState<Set<number>>(new Set()); | ||
|
||
const handleInform = () => { | ||
localStorage.setItem("user:isInformed", "true"); |
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.
maybe user:isExtensionsWarningShown
?
it("renders all accordion items", async () => { | ||
await renderInModal(<SecurityWarningModal />); | ||
|
||
accordionItems.forEach(({ title }) => { |
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.
nit: can we have an array of titles here instead?
If there is a fault with the accordionItems
this test will still be green.
expect(gotItButton).toBeEnabled(); | ||
}); | ||
|
||
it("sets localStorage and closes modal when 'Got it' is clicked", async () => { |
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.
nit: would be better to split into two separate tests, the more simple unit test is - the better
8daea34
to
26f9a2f
Compare
Proposed changes
Task link
Types of changes
Steps to reproduce
Screenshots
Checklist