Skip to content
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 tests for Footer component #397

Merged

Conversation

harisbikovic
Copy link
Contributor

Related #390
This PR containes tests for the Footer component.

@szakhlypa
Copy link
Contributor

I prefer to not depend on link's order.
What if we add some more accessibility into component with aria-label and use it in getByRole?
For example:
repository link in Footer.tsx:

  <a
    href={repoUrl}
    _other_props_here_
    aria-label={
      intl.formatMessage({
        id: "components.Footer.repositoryLinkLabel",
        defaultMessage: "GitHub",
      })
    }
  >
    <Icon icon="github" />
  </a>

And its test with getByRole

  const repositoryLink = screen.getByRole("link", { name: "GitHub" });
  expect(repositoryLink).toHaveAttribute("href", "https://github.com/edgehog-device-manager/edgehog");

Also, can we use different values for homepageUrl and repoUrl props in test to distinguish in case of error.

@harisbikovic harisbikovic force-pushed the footer_test_branch branch 2 times, most recently from 0da6db7 to 0ffb323 Compare October 23, 2023 06:21
@harisbikovic harisbikovic marked this pull request as ready for review October 30, 2023 09:18
frontend/src/components/Footer.test.tsx Show resolved Hide resolved
frontend/src/components/Footer.test.tsx Show resolved Hide resolved
frontend/src/i18n/langs/en.json Outdated Show resolved Hide resolved
@harisbikovic harisbikovic force-pushed the footer_test_branch branch 2 times, most recently from 1b45089 to 8bd27d5 Compare November 6, 2023 09:40
frontend/src/components/Footer.test.tsx Outdated Show resolved Hide resolved
frontend/src/components/Footer.test.tsx Outdated Show resolved Hide resolved
frontend/src/components/Footer.test.tsx Outdated Show resolved Hide resolved
frontend/src/components/Footer.tsx Outdated Show resolved Hide resolved
frontend/src/i18n/langs/en.json Outdated Show resolved Hide resolved
@harisbikovic harisbikovic force-pushed the footer_test_branch branch 15 times, most recently from 2c69a31 to 6535f06 Compare November 6, 2023 14:58
@harisbikovic harisbikovic marked this pull request as draft November 9, 2023 11:41
@harisbikovic harisbikovic force-pushed the footer_test_branch branch 3 times, most recently from 017f8a0 to e2b48aa Compare November 10, 2023 09:41
@harisbikovic harisbikovic force-pushed the footer_test_branch branch 3 times, most recently from c6e5dc4 to 173202f Compare November 10, 2023 12:38
@harisbikovic harisbikovic marked this pull request as ready for review November 10, 2023 15:08
frontend/src/components/Footer.test.tsx Outdated Show resolved Hide resolved
frontend/src/components/Footer.test.tsx Outdated Show resolved Hide resolved
frontend/src/components/Footer.test.tsx Outdated Show resolved Hide resolved
const appNameElement = screen.getByText("Edgehog Device Manager");
expect(appNameElement).toBeVisible();

const appVersionElement = screen.getByText(/0.1.0-alpha.1/);
Copy link
Contributor

Choose a reason for hiding this comment

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

Dot in RegExp matches any single character except line terminators, so we need to escape it with backslash. Otherwise test will pass even with appVersion="0x1x0-alphax1" property.

Suggested change
const appVersionElement = screen.getByText(/0.1.0-alpha.1/);
const appVersionElement = screen.getByText(/0\.1\.0-alpha\.1/);

frontend/src/components/Footer.tsx Show resolved Hide resolved
@harisbikovic harisbikovic force-pushed the footer_test_branch branch 2 times, most recently from bda286a to e976d24 Compare November 15, 2023 08:40
Add acessibility labels for icon only links.

Signed-off-by: Haris Bikovic <[email protected]>
Generated with `npm run i18n:extract`

Signed-off-by: Haris Bikovic <[email protected]>
Signed-off-by: Haris Bikovic <[email protected]>
@szakhlypa szakhlypa merged commit 2309cf5 into edgehog-device-manager:release-0.7 Nov 30, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants