-
Notifications
You must be signed in to change notification settings - Fork 208
Add an FAQ entry on acceptable use of Relay #1541
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
Conversation
frontend/src/pages/faq.page.tsx
Outdated
<Localized | ||
id="faq-question-acceptable-use-answer-a-html" | ||
vars={{ | ||
url: "https://www.mozilla.org/about/legal/acceptable-use/", | ||
attrs: "", | ||
}} | ||
elems={{ | ||
a: ( | ||
<a href="https://www.mozilla.org/about/legal/acceptable-use/" /> | ||
), | ||
}} | ||
> | ||
<p /> | ||
</Localized> |
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.
This is a small peculiarity of @fluent/react
: it can only replace <tags>
in translation strings via the <Localized>
wrapper component, not via the useLocalization
hook (that provides the l10n
object).
The way that component works is: it takes its child element (<p />
, in this case), and replaces its contents with the contents of the Fluent string (faq-question-acceptable-use-answer-a-html
).
Also note that vars
shouldn't actually be necessary: if the string just says <my-tag>acceptable use policy</my-tag>
, then we could add my-tag
to elems
and set whatever attribute we need on the element there, just like we set the href
attribute on the <a>
element we replace the a
tag with. However, because this string is also used on the current site, I had to use the <a href="{ $url }" { $attrs }>
trick too.
9819589
to
6e70122
Compare
6e70122
to
a2a28d0
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.
I have one non-blocking change!
frontend/src/pages/faq.page.tsx
Outdated
}} | ||
elems={{ | ||
a: ( | ||
<a href="https://www.mozilla.org/about/legal/terms/firefox-relay/" /> |
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.
suggestion: Can open all FAQ links in a new tab?
From the Djagon frontend, we add the following attrs to each link in the FAQ: https://github.com/mozilla/fx-private-relay/blob/main/static/js/app.js#L362-L373
Note that the noopener
may no longer be necessary.
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.
Note that the linter (configured to use the recommended default rules by ESLint and Next.js) added both rel
s automatically on commit. Better safe than sorry, I suppose :)
New feature description
Like #1540, but applied to the React codebase from #1518 (this PR merges into that PR's branch). To get ourselves used to reviewing React PRs 🙂
Screenshot (if applicable)
How to test
Go to the
/faq
page, verify that the new question is shown there.Checklist
/static/scss/libs/protocol/css/includes/tokens/dist/index.scss
).