Skip to content

Commit

Permalink
Merge pull request #2216 from flacial/2215-fix-cant-dismiss-an-alert
Browse files Browse the repository at this point in the history
fix: Alert can't be dismissed
  • Loading branch information
flacial authored Aug 28, 2022
2 parents a39a411 + 5f31a3b commit fcd0336
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 132 deletions.
50 changes: 18 additions & 32 deletions __tests__/__snapshots__/storyshots.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ exports[`Storyshots Components/AdminLessonSideNav Basic 1`] = `

exports[`Storyshots Components/Alert Info 1`] = `
<div
className="alert d-flex justify-content-between mt-3 alert alert-primary"
className="fade alert alert-primary show"
role="alert"
>
<div
Expand All @@ -931,9 +931,15 @@ exports[`Storyshots Components/Alert Info 1`] = `

exports[`Storyshots Components/Alert Info With Link And Dismiss 1`] = `
<div
className="alert d-flex justify-content-between mt-3 alert alert-primary"
className="fade alert alert-primary alert-dismissible show"
role="alert"
>
<button
aria-label="Close alert"
className="btn-close"
onClick={[Function]}
type="button"
/>
<div
className="d-flex align-items-center gap-3"
>
Expand All @@ -957,25 +963,12 @@ exports[`Storyshots Components/Alert Info With Link And Dismiss 1`] = `
View Instructions
</a>
</div>
<button
className="info_image alert-dismiss"
data-testid="dismiss-info"
onClick={[Function]}
role="dismiss"
>
<img
className="img-fluid"
height={24}
src="/assets/curriculum/icons/dismiss-info.svg"
width={24}
/>
</button>
</div>
`;

exports[`Storyshots Components/Alert Urgent 1`] = `
<div
className="alert d-flex justify-content-between mt-3 alert alert-danger"
className="fade alert alert-danger show"
role="alert"
>
<div
Expand All @@ -998,9 +991,15 @@ exports[`Storyshots Components/Alert Urgent 1`] = `

exports[`Storyshots Components/Alert Urgent With Icon And Dismiss 1`] = `
<div
className="alert d-flex justify-content-between mt-3 alert alert-danger"
className="fade alert alert-danger alert-dismissible show"
role="alert"
>
<button
aria-label="Close alert"
className="btn-close"
onClick={[Function]}
type="button"
/>
<div
className="d-flex align-items-center gap-3"
>
Expand All @@ -1024,19 +1023,6 @@ exports[`Storyshots Components/Alert Urgent With Icon And Dismiss 1`] = `
View NPM Package
</a>
</div>
<button
className=" alert-dismiss"
data-testid="dismiss-urgent"
onClick={[Function]}
role="dismiss"
>
<img
className="img-fluid"
height={24}
src="/assets/curriculum/icons/dismiss-urgent.svg"
width={24}
/>
</button>
</div>
`;

Expand Down Expand Up @@ -28050,7 +28036,7 @@ exports[`Storyshots Pages/Login Login Error 1`] = `
className="form-group"
>
<div
className="alert d-flex justify-content-between mt-3 alert alert-danger"
className="fade alert alert-danger show"
role="alert"
>
<div
Expand Down Expand Up @@ -28527,7 +28513,7 @@ exports[`Storyshots Pages/Signup Signup Errors 1`] = `
className="card-text"
/>
<div
className="alert d-flex justify-content-between mt-3 alert alert-danger"
className="fade alert alert-danger show"
role="alert"
>
<div
Expand Down
2 changes: 1 addition & 1 deletion __tests__/pages/__snapshots__/login.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ exports[`Login Page Should set alert visible on invalid credentials 1`] = `
class="form-group"
>
<div
class="alert d-flex justify-content-between mt-3 alert alert-danger"
class="fade alert alert-danger show"
role="alert"
>
<div
Expand Down
2 changes: 1 addition & 1 deletion __tests__/pages/__snapshots__/signup.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ exports[`Signup Page Should render errors on fail 1`] = `
class="card-text"
/>
<div
class="alert d-flex justify-content-between mt-3 alert alert-danger"
class="fade alert alert-danger show"
role="alert"
>
<div
Expand Down
72 changes: 24 additions & 48 deletions __tests__/pages/curriculum/__snapshots__/[lessonSlug].test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,14 @@ exports[`Lesson Page Should correctly render challenges page for students who ha
class="alerts-container p-0"
>
<div
class="alert d-flex justify-content-between mt-3 alert alert-primary"
class="fade alert alert-primary alert-dismissible show"
role="alert"
>
<button
aria-label="Close alert"
class="btn-close"
type="button"
/>
<div
class="d-flex align-items-center gap-3"
>
Expand All @@ -246,22 +251,16 @@ exports[`Lesson Page Should correctly render challenges page for students who ha
View Instructions
</a>
</div>
<button
class="info_image alert-dismiss"
data-testid="dismiss-info"
role="dismiss"
>
<img
height="24"
src="/assets/curriculum/icons/dismiss-info.svg"
width="24"
/>
</button>
</div>
<div
class="alert d-flex justify-content-between mt-3 alert alert-danger"
class="fade alert alert-danger alert-dismissible show"
role="alert"
>
<button
aria-label="Close alert"
class="btn-close"
type="button"
/>
<div
class="d-flex align-items-center gap-3"
>
Expand All @@ -277,17 +276,6 @@ exports[`Lesson Page Should correctly render challenges page for students who ha
Please upgrade your CLI client by running npm update c0d3.
</div>
<button
class=" alert-dismiss"
data-testid="dismiss-urgent"
role="dismiss"
>
<img
height="24"
src="/assets/curriculum/icons/dismiss-urgent.svg"
width="24"
/>
</button>
</div>
</div>
<div
Expand Down Expand Up @@ -933,9 +921,14 @@ exports[`Lesson Page Should render correctly with valid lesson route 1`] = `
class="alerts-container p-0"
>
<div
class="alert d-flex justify-content-between mt-3 alert alert-primary"
class="fade alert alert-primary alert-dismissible show"
role="alert"
>
<button
aria-label="Close alert"
class="btn-close"
type="button"
/>
<div
class="d-flex align-items-center gap-3"
>
Expand All @@ -958,22 +951,16 @@ exports[`Lesson Page Should render correctly with valid lesson route 1`] = `
View Instructions
</a>
</div>
<button
class="info_image alert-dismiss"
data-testid="dismiss-info"
role="dismiss"
>
<img
height="24"
src="/assets/curriculum/icons/dismiss-info.svg"
width="24"
/>
</button>
</div>
<div
class="alert d-flex justify-content-between mt-3 alert alert-danger"
class="fade alert alert-danger alert-dismissible show"
role="alert"
>
<button
aria-label="Close alert"
class="btn-close"
type="button"
/>
<div
class="d-flex align-items-center gap-3"
>
Expand All @@ -989,17 +976,6 @@ exports[`Lesson Page Should render correctly with valid lesson route 1`] = `
Please upgrade your CLI client by running npm update c0d3.
</div>
<button
class=" alert-dismiss"
data-testid="dismiss-urgent"
role="dismiss"
>
<img
height="24"
src="/assets/curriculum/icons/dismiss-urgent.svg"
width="24"
/>
</button>
</div>
</div>
<div
Expand Down
28 changes: 28 additions & 0 deletions components/Alert/Alert.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import React from 'react'
import { render, screen } from '@testing-library/react'
import Alert from '.'
import userEvent from '@testing-library/user-event'

// Imported to be able to use .toBeInTheDocument()
import '@testing-library/jest-dom'

describe('Alert component', () => {
it('should hide the alert on dismiss', async () => {
render(
<Alert
alert={{
id: 0,
text: 'Set up your computer to submit challenges.',
type: 'info',
url: 'https://www.notion.so/JS-0-Foundations-a43ca620e54945b2b620bcda5f3cf672#b45ed85a95e24c9d9fb784afb7a46bcc',
urlCaption: 'View Instructions'
}}
onDismiss={() => {}}
/>
)

await userEvent.click(screen.getByRole('button'))

expect(screen.queryByRole('alert')).not.toBeInTheDocument()
})
})
37 changes: 15 additions & 22 deletions components/Alert/Alert.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import React from 'react'
import React, { useState } from 'react'
import NavLink from '../NavLink'
import { Alert as AlertType } from '../../graphql/'
import _ from 'lodash'
import styles from './alerts.module.scss'
import Image from 'next/image'
import { Alert as AlertBS } from 'react-bootstrap'

type Props = {
alert: AlertType
Expand All @@ -21,16 +22,20 @@ const alertIconMap: { [type: string]: string } = {
}

const Alert: React.FC<Props> = ({ alert, onDismiss }) => {
const [show, setShow] = useState(true)

const { text, type, url, urlCaption, id } = alert
const icon = alertIconMap[_.defaultTo(type, '-1')]
const alertClasses =
type === 'urgent' ? 'alert alert-danger' : `alert alert-primary`
const imageClass = type === 'urgent' ? '' : styles.info_image

return (
<div
className={`${styles.alert} d-flex justify-content-between mt-3 ${alertClasses}`}
role="alert"
return show ? (
<AlertBS
variant={`${type === 'urgent' ? 'danger' : 'primary'}`}
onClose={() => {
setShow(false)
onDismiss && onDismiss(id)
}}
dismissible={!!onDismiss}
>
<div className="d-flex align-items-center gap-3">
<div className={imageClass}>
Expand All @@ -43,21 +48,9 @@ const Alert: React.FC<Props> = ({ alert, onDismiss }) => {
</NavLink>
)}
</div>
{onDismiss && (
<button
role="dismiss"
className={`${imageClass} ${styles['alert-dismiss']}`}
data-testid={`dismiss-${type}`}
onClick={() => onDismiss(id)}
>
<Image
src={`/assets/curriculum/icons/dismiss-${type}.svg`}
width={24}
height={24}
/>
</button>
)}
</div>
</AlertBS>
) : (
<></>
)
}

Expand Down
8 changes: 4 additions & 4 deletions components/AlertsDisplay/AlertsDisplay.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('Alerts Display Component', () => {
<AlertsDisplay alerts={alerts} page="curriculum" />
)

const displayedAlerts = getAllByRole('alert')
const displayedAlerts = getAllByRole('button')
expect(displayedAlerts.length).toEqual(1)
})

Expand All @@ -48,13 +48,13 @@ describe('Alerts Display Component', () => {
const { getAllByRole } = render(
<AlertsDisplay alerts={alerts} page="curriculum" />
)
let displayedAlerts = getAllByRole('alert')
let displayedAlerts = getAllByRole('button')
expect(displayedAlerts.length).toEqual(2)
const firstDismiss = getAllByRole('dismiss')[0]
const firstDismiss = getAllByRole('button')[0]
act(() => {
fireEvent.click(firstDismiss)
})
displayedAlerts = getAllByRole('alert')
displayedAlerts = getAllByRole('button')
expect(displayedAlerts.length).toEqual(1)
})
})
Loading

1 comment on commit fcd0336

@vercel
Copy link

@vercel vercel bot commented on fcd0336 Aug 28, 2022

Choose a reason for hiding this comment

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

Please sign in to comment.