Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
403 changes: 214 additions & 189 deletions __tests__/__snapshots__/snapshot.js.snap

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions __tests__/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ it('renders homepage unchanged', () => {
version: '0.0',
currentApprentices: [
{
name: 'First Last',
name: 'First Last1',
image: '/apprentices/First.png',
links: [
{ href: 'personal', text: 'Personal' },
Expand All @@ -17,7 +17,7 @@ it('renders homepage unchanged', () => {
],
},
{
name: 'First Last',
name: 'First Last2',
image: '/apprentices/First.png',
links: [
{ href: 'personal', text: 'Personal' },
Expand All @@ -31,14 +31,14 @@ it('renders homepage unchanged', () => {
version: '0.0',
apprentices: [
{
name: 'First Last',
name: 'First Last1',
},
{
name: 'First Last',
name: 'First Last2',
status: 'previous',
},
{
name: 'First Last',
name: 'First Last3',
status: 'current',
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,18 @@ const ApprenticeStatusIndicator = ({ status }) => {
}

return (
<svg viewBox="0 0 100 100" className={styles['employee-status']}>
<polygon
points="50 15, 100 100, 0 100"
className={styles[svgClass]}
data-testid="status-indicator"
/>
</svg>
<>
<span className={styles['status-label']}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

{ `${status} employee` }
</span>
<svg viewBox="0 0 100 100" className={styles['employee-status']} aria-hidden>
<polygon
points="50 15, 100 100, 0 100"
className={styles[svgClass]}
data-testid="status-indicator"
/>
</svg>
</>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
@use '../../styles/colors' as *;
@use '../../styles/spacing' as *;
@use '../../styles/fonts' as *;
@use '../../styles/mixins' as *;

.status-label {
@include visually-hidden();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just making a note: Let's be sure to coordinate this merge with #31 since it includes the extracted mixin


.employee-status {
width: 0.75rem;
Expand Down
10 changes: 5 additions & 5 deletions components/apprentice/apprentice.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import SocialLinks, { linksPropTypes } from '../social-links/social-links';
import styles from './apprentice.module.scss';

const Apprentice = ({ image, name, links }) => (
<article className={styles.container}>
<h2 className={styles.container__heading}>{ name }</h2>
<img className={styles.container__image} src={image} alt={name} />
<div className={styles.container}>
<h3 className={styles.container__heading}>{ name }</h3>
<img className={styles.container__image} src={image} alt={`a portrait of ${name}`} />
<div className={styles.container__links}>
<SocialLinks links={links} />
<SocialLinks links={links} name={name} />
</div>
</article>
</div>
);

export const apprenticePropTypes = {
Expand Down
2 changes: 1 addition & 1 deletion components/apprentice/apprentice.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('the Apprentice component', () => {
});

it('adds name prop as alt attribute to image', () => {
expect(image).toHaveAttribute('alt', 'First Last');
expect(image).toHaveAttribute('alt', 'a portrait of First Last');
});

it('renders links props given an href and text in the correct order', () => {
Expand Down
2 changes: 1 addition & 1 deletion components/call-to-action/call-to-action.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@

&__grid {
width: 90%;
grid-template-columns: repeat(67, 1fr);
grid-template-columns: repeat(65, 1fr);
grid-template-rows: repeat(13, 1fr);

img {
Expand Down
4 changes: 1 addition & 3 deletions components/hero/hero.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@

&__grid { // This grid is for the hero images and svgs
display: grid;
grid-template-columns: repeat(65, 1fr);
grid-template-columns: repeat(63, 1fr);
grid-template-rows: repeat(11, 1fr);

img {
Expand Down Expand Up @@ -212,7 +212,6 @@

&__sparkbox-logo {
&--full {
padding: 1rem;
margin-bottom: 0;
}
}
Expand All @@ -224,7 +223,6 @@
&__text {
width: 100%;
order: 0;
padding: 1rem;
margin-top: 2rem;

h1 {
Expand Down
5 changes: 3 additions & 2 deletions components/link/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ import React from 'react';
import PropTypes from 'prop-types';
import styles from './link.module.scss';

const Link = ({ href, children }) => (
<a className={styles.link} href={href}>{children}</a>
const Link = ({ href, ariaLabel, children }) => (
<a className={styles.link} href={href} aria-label={ariaLabel}>{children}</a>
);

Link.propTypes = {
href: PropTypes.string.isRequired,
ariaLabel: PropTypes.string.isRequired,
children: PropTypes.oneOfType([PropTypes.string, PropTypes.element]).isRequired,
};

Expand Down
11 changes: 8 additions & 3 deletions components/link/link.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,17 @@ import Link from './link';

describe('the Link component', () => {
it('renders a link with the correct href given an href prop', () => {
render(<Link href="http://some/url">hello!</Link>);
render(<Link href="http://some/url" ariaLabel="test link">hello</Link>);
expect(screen.getByRole('link')).toHaveAttribute('href', 'http://some/url');
});

it('renders a link given an href and text', () => {
render(<Link href="http://some/url">hello!</Link>);
it('renders a link with the correct aria-label given an ariaLabel prop', () => {
render(<Link href="http://some/url" ariaLabel="test link">hello</Link>);
expect(screen.getByRole('link')).toHaveAttribute('aria-label', 'test link');
});

it('renders a link with the correct inner text', () => {
render(<Link href="http://some/url" ariaLabel="test link">hello</Link>);
expect(screen.getByRole('link')).toHaveTextContent('hello');
});
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it possible to test the new label?

Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ const makePreviousApprentice = (apprentice) => (

const PreviousApprenticesGroup = ({ version, apprentices }) => (
<section className={styles['previous-apprentices-group']}>
<p
<h3
className={styles['previous-apprentices-group__version']}
aria-label={`Previous apprentices group version ${version}`}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I love this.

data-testid={version}
>
{version}
</p>
</h3>
<ul className={styles['previous-apprentices-group__statuses']}>
{apprentices.map(makePreviousApprentice)}
</ul>
Expand Down
8 changes: 4 additions & 4 deletions components/previous-apprentices/previous-apprentices.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import styles from './previous-apprentices.module.scss';
import PreviousApprenticesGroup from '../previous-apprentices-group/previous-apprentices-group';

const PreviousApprentices = ({ previousApprenticeGroups }) => (
<article className={styles['previous-apprentices']}>
<section className={styles['previous-apprentices__header']}>
<section className={styles['previous-apprentices']}>
<div className={styles['previous-apprentices__header']}>
<h2>Previous Apprentices</h2>
<div className={styles['previous-apprentices__legend']}>
<div className={styles['previous-apprentices__legend-item']}>
Expand All @@ -27,7 +27,7 @@ const PreviousApprentices = ({ previousApprenticeGroups }) => (
<p>Past Employee</p>
</div>
</div>
</section>
</div>
<div className={styles['previous-apprentices__groups']}>
{
previousApprenticeGroups.map(({ version, apprentices }) => (
Expand All @@ -40,7 +40,7 @@ const PreviousApprentices = ({ previousApprenticeGroups }) => (
))
}
</div>
</article>
</section>
);

export const previousApprenticeGroupsPropTypes = PropTypes.shape({
Expand Down
7 changes: 4 additions & 3 deletions components/social-links/social-links.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import PropTypes from 'prop-types';
import Link from '../link/link';
import styles from './social-links.module.scss';

const SocialLinks = ({ links }) => (
const SocialLinks = ({ links, name }) => (
<>
{ links.map(({ href, text }) => (
<div className={styles.container} key={href}>
<Link href={href}>{ text }</Link>
<svg width="11" height="9" viewBox="0 0 11 9" fill="none" xmlns="http://www.w3.org/2000/svg">
<Link href={href} ariaLabel={`${name}'s ${text}`}>{ text }</Link>
<svg width="11" height="9" viewBox="0 0 11 9" fill="none" xmlns="http://www.w3.org/2000/svg" aria-hidden>
<path fillRule="evenodd" clipRule="evenodd" d="M0.5 3.07343L0.5 8.5L1.26923 8.5L1.26923 3.07343L8.31903 3.07343C7.95887 3.47125 7.57203 4.06799 7.15851 4.86364L7.85883 4.86364C8.6992 3.94367 9.57959 3.27234 10.5 2.84965L10.5 2.51399C9.57959 2.07887 8.6992 1.40754 7.85883 0.500001L7.15852 0.500001C7.57203 1.29565 7.95887 1.89239 8.31903 2.29021L0.5 2.29021L0.5 2.68182L0.5 3.07343Z" fill="white" />
</svg>
</div>
Expand All @@ -23,6 +23,7 @@ export const linksPropTypes = PropTypes.arrayOf(PropTypes.shape({

SocialLinks.propTypes = {
links: linksPropTypes,
name: PropTypes.string.isRequired,
};

SocialLinks.defaultProps = {
Expand Down
17 changes: 12 additions & 5 deletions components/social-links/social-links.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,33 @@ import SocialLinks from './social-links';

describe('the Link component', () => {
it('renders nothing when passed empty array', async () => {
const { container } = render(<SocialLinks links={[]} />);
const { container } = render(<SocialLinks links={[]} name="name" />);
expect(container.innerHTML).toBe('');
});

it('renders nothing when links is not defined', async () => {
const { container } = render(<SocialLinks />);
const { container } = render(<SocialLinks name="name" />);
expect(container.innerHTML).toBe('');
});

it('renders links given an href and text in the correct order', () => {
it('renders multiple social links in the correct order', () => {
render(<SocialLinks
links={[
{ href: 'https://www.facebook.com/', text: 'facebook' },
{ href: 'https://www.google.com/', text: 'google' },
]}
name="name"
/>);
const links = screen.getAllByRole('link');
expect(links[0]).toHaveAttribute('href', 'https://www.facebook.com/');
expect(links[0]).toHaveTextContent('facebook');
expect(links[1]).toHaveAttribute('href', 'https://www.google.com/');
expect(links[1]).toHaveTextContent('google');
});

it('renders a social link', () => {
render(<SocialLinks links={[{ href: 'https://www.linkedin.com/', text: 'LinkedIn' }]} name="First Last" />);
const link = screen.getByRole('link');
expect(link).toHaveTextContent('LinkedIn');
expect(link).toHaveAttribute('href', 'https://www.linkedin.com/');
expect(link).toHaveAttribute('aria-label', "First Last's LinkedIn");
});
});
3 changes: 1 addition & 2 deletions pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ const Home = ({ apprenticeData: { currentApprenticeGroup, previousApprenticeGrou
<title>Sparkbox Apprentices</title>
<link rel="icon" href="/favicon.ico" />
</Head>

<Hero />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, easy fix

<main>
<Hero />
<ApprenticeQualities />
<CurrentApprentices currentApprenticeClass={currentApprenticeGroup} />
<PreviousApprentices previousApprenticeGroups={previousApprenticeGroups} />
Expand Down