-
Notifications
You must be signed in to change notification settings - Fork 178
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
feat(protocol-designer, components): analytics opt-in modal and footer #16561
Conversation
@jerader this is awesome! I think the button copy is swapped though. "Agree" should be the blue button on the right and "Reject" should be the sub-button on the left. |
// TODO(ja: 10/22/24): figure out the i18n stuff since components package.json does not have i18n and | ||
// should not have i18n (since other projects use it). Due to the links, we would need access to both t and Trans |
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.
I guess we don't need to do this since basically text is passed to a component in ui components.
From my experience, if the components aren't published, looking for the way would be important but opentrons ui components are published so theoretically we should avoid using fixed text inside ui components.
I think this component itself is totally fine.
// should not have i18n (since other projects use it). Due to the links, we would need access to both t and Trans | ||
export function EndUserAgreementFooter(): JSX.Element { | ||
return ( | ||
<Flex |
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.
I think wrapping this by footer or creating a Footer with styled-component would be good.
const Footer = styled(footer)`
background-color: ${COLORS.grey20}
....
`
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.
left a couple of comments but overall lgtm
9055a23
to
b6aa06a
Compare
closes AUTH-967, AUTH-888, AUTH-792
Overview
Update the analytics opt-in modal, add it to the redesign, and add the footer to the landing page and overview page
Screen.Recording.2024-10-23.at.13.05.57.mov
Test Plan and Hands on Testing
see the analytics modal and the footer in the landing and overview pages
Changelog
Risk assessment
low