-
Notifications
You must be signed in to change notification settings - Fork 215
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 the ToS to the navigation bar #3727
Conversation
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.
Awesome PR with updated snapshots 🎉
You will also need to update the storybook snapshots that test header and footer in isolation. just frontend/run test:storybook -u
is probably the command that does so.
7cb9379
to
2382aed
Compare
Thanks @obulat for that tip! I'll add it to the frontend documentation on testing as well (that's what I was following 😄) |
I'm...not sure how to proceed now 😅 |
2382aed
to
afad3e3
Compare
@AetherUnbound What are you stuck on? The failing Playwright test in CI might be a flaky test failure, in which case follow this from the comment above. |
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 can't see any difference in frontend/test/storybook/visual-regression/v-image-cell.spec.ts-snapshots/v-image-cell-intrinsic-focused-xl-linux.png
but if CI passes then it's okay I guess?
Might I suggest two explorations?
- updating this PR branch with
main
- dropping that file from the PR and seeing if tests continue to pass
afad3e3
to
11b6f58
Compare
I was able to rebase on main, but re-running storybook locally is adding even more changes that don't seem like they're significant 😕 The fact that CI is passing here on main after a rebase makes me think things are fine, so I'm just going to merge this! |
Fixes
Fixes #3564 by @AetherUnbound
Description
This PR adds a link to the Terms of Service (using the name "Terms") to the navigation bar. This is a stopgap measure while we do a larger reorganization of the navigation menu.
There might be something else I'm missing with this PR, frontend folks - please let me know if that's the case!
Testing Instructions
I've updated the playwright snapshots. I think I've done everything that's necessary for testing!
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin