Refactor footer #1561 #1572
Refactor footer #1561 #1572
Conversation
JabaDUDE
requested review from
sashamaryl,
mertbagt,
nesanders,
mvictor55,
timblais,
alexjball and
Mephistic
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
components/Footer/Footer.tsx
Outdated
| @@ -193,13 +168,13 @@ const LearnLinks = () => { | |||
| return ( | |||
| <> | |||
| <StyledInternalLink href="/learn/writing-effective-testimony"> | |||
| {t("links.learnWriting")} | |||
| {t("To Writing Effective Testimony")} | |||
There was a problem hiding this comment.
nit: The param passed to the t translate function should be a key pointing to the relevant text in the locales/** language files, not the english text itself. The goal is to have no user-visible text in component files, including the english text, and we only reference user-visible text based on their translation keys.
e.g. Instead of t("To Writing Effective Testimony"), this should be something like t("common.footer.writingTestimony") - and the locales/en/common.json file should contain a key for common.footer.writingTestimony with the value of `"To Writing Effective Testimony".
micro nit: To Writing Effective Testimony should probably just be Writing Effective Testimony
|
|
||
| &:hover { |
There was a problem hiding this comment.
In the Desktop view, the Browse Bills text gets unreadably dark when hovered over - we should probably add a hover style to make this more visible, not less when a user is ready to interact with it.
There was a problem hiding this comment.
On hover, this is what the Browse Bills link looks like:
Summary
Checklist
Screenshots
Known issues
Only caveat is the spacing of the footer on desktop. Might need to redo footer in columns and rows to create even spaces between bottom and top half of footer
Steps to test/reproduce
For each feature or bug fix, create a step by step list for how a reviewer can test it out. E.g.: