Front End from Nava next.js template #283
Conversation
| @@ -0,0 +1,26 @@ | |||
| # Use Markdown Architectural Decision Records | |||
There was a problem hiding this comment.
Should all these ADRs be moved into our ADR directory and merged with our ADRs in terms of numbering, etc?
There was a problem hiding this comment.
Sure, I'll move those over.
.grype.yml
Outdated
| # High severity | ||
| # Ignoring since this is only a dependency of dev tools: storybook (for storybook docs site), | ||
| # eslint (for Linting in CI), and sass (for compiling CSS during CI build phase) | ||
| - vulnerability: GHSA-ww39-953v-wcq6 |
There was a problem hiding this comment.
Are these vulnerabilities still live?
There was a problem hiding this comment.
Good call, I just double checked and it seems like they are no longer in the project.
There was a problem hiding this comment.
Also, noting that when we add infrastructure via terraform, we'll have a github workflow that incorporates the grype scanning in the ci/cd pipeline.
There was a problem hiding this comment.
TIL anchore scan is grype
|
|
||
| import Header from "src/components/Header"; | ||
|
|
||
| describe("Header", () => { |
There was a problem hiding this comment.
we love to see front-end tests!
| expect(header).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("passes accessibility scan", async () => { |
| require("@testing-library/jest-dom"); | ||
| const { toHaveNoViolations } = require("jest-axe"); | ||
|
|
||
| expect.extend(toHaveNoViolations); |
There was a problem hiding this comment.
These changes look great to me, thanks @SammySteiner .
My review shouldn't "count" as a strong approval for you because I'm no longer sharp enough to catch vulnerabilities etc, but it looks good to me!
🚀 A couple comments on small things like broken links
|
|
||
| - [I18next](https://www.i18next.com/) is used for internationalization. | ||
| - Next.js's [internationalized routing](https://nextjs.org/docs/advanced-features/i18n-routing) feature is enabled. Toggling between languages is done by changing the URL's path prefix (e.g. `/about` ➡️ `/es/about`). | ||
| - Configuration for the i18n routing and i18next libraries are located in [`next-i18next.config.js`](../app/next-i18next.config.js). |
There was a problem hiding this comment.
This local file link needs to be updated, it is broken
| 1. Edit `next-i18next.config.js` and add the language to `locales`, using the BCP47 language tag (e.g. `en` or `es`). | ||
| 1. Add a language folder, using the same BCP47 language tag: `mkdir -p public/locales/<lang>` | ||
| 1. Add a language file: `touch public/locales/<lang>/common.json` and add the translated content. The JSON structure should be the same across languages. However, non-default languages can omit keys, in which case the default language will be used as a fallback. | ||
| 1. Optionally, add a label for the language to the `locales` object in [`.storybook/preview.js`](../app/.storybook/preview.js) |
There was a problem hiding this comment.
This file path needs to be updated, broken too
Summary
Fixes #110
Time to review: 30 minutes
Changes proposed
Context for reviewers