Skip to content

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

Merged
merged 2 commits into from
Feb 16, 2022

Conversation

Vinnl
Copy link
Collaborator

@Vinnl Vinnl commented Feb 14, 2022

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)

image

How to test

Go to the /faq page, verify that the new question is shown there.

Checklist

@Vinnl Vinnl self-assigned this Feb 14, 2022
Comment on lines 238 to 251
<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>
Copy link
Collaborator Author

@Vinnl Vinnl Feb 14, 2022

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.

@maxxcrawford maxxcrawford added the react Temporary label: This issue/PR is on the React front end label Feb 14, 2022
@maxxcrawford
Copy link
Contributor

Quick note for @codemist and @Vinnl: I created a new label for any React focused issues or issues while we are managing both front ends. If we need a label for the Django front-end, lmk and we can make another tag too.

@Vinnl Vinnl force-pushed the MPP-1589-abuse-faq-react branch from 9819589 to 6e70122 Compare February 14, 2022 16:53
@Vinnl Vinnl force-pushed the MPP-1589-abuse-faq-react branch from 6e70122 to a2a28d0 Compare February 15, 2022 08:54
Copy link
Contributor

@maxxcrawford maxxcrawford left a 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!

}}
elems={{
a: (
<a href="https://www.mozilla.org/about/legal/terms/firefox-relay/" />
Copy link
Contributor

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.

Copy link
Collaborator Author

@Vinnl Vinnl Feb 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7b1a05b

Note that the linter (configured to use the recommended default rules by ESLint and Next.js) added both rels automatically on commit. Better safe than sorry, I suppose :)

@Vinnl Vinnl merged commit 1fce14f into react-experiment Feb 16, 2022
@Vinnl Vinnl deleted the MPP-1589-abuse-faq-react branch February 16, 2022 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
react Temporary label: This issue/PR is on the React front end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants