Skip to content

Commit

Permalink
EDD-31: PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
trevorlang authored and macrouch committed Oct 13, 2023
1 parent 70f5f84 commit 8c0b50b
Show file tree
Hide file tree
Showing 16 changed files with 127 additions and 112 deletions.
72 changes: 1 addition & 71 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@
"react-icons": "^4.8.0",
"react-middle-ellipsis": "^1.2.2",
"react-virtualized-auto-sizer": "^1.0.20",
"react-window": "^1.8.9",
"simplebar-react": "^3.2.4"
"react-window": "^1.8.9"
},
"devDependencies": {
"@babel/cli": "^7.21.0",
Expand Down
1 change: 0 additions & 1 deletion src/app/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import Layout from './components/Layout/Layout'

import Providers from './providers/Providers/Providers'

import 'simplebar-react/dist/simplebar.min.css'
import 'inter-ui/inter.css'

import './App.scss'
Expand Down
7 changes: 4 additions & 3 deletions src/app/components/Button/Button.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import createVariantClassName from '../../utils/createVariantClassName'
* @property {String} [rel] An optional string to be used as the `rel` attribute on a link. If passed, the button will be rendered as an <a> element.
* @property {String} [size] An optional string which modifies the size of the button.
* @property {String} [target] An optional string to be used as the `target` attribute on a link.
* @property {('slow')} [tooltipDelayDuration] An optional variant which modifies the visual appearance of the button.
* @property {String} [variant] An optional variant which modifies the visual appearance of the button.
* @property {...*} [rest] Any additional props will be spread as props onto the root element, which is important for allowing tooltips and other enhancements.
*/
Expand Down Expand Up @@ -133,7 +134,7 @@ const Button = forwardRef(({
return (
<Tooltip
content={children}
delayDuration={tooltipDelayDuration}
duration={tooltipDelayDuration}
>
{button}
</Tooltip>
Expand Down Expand Up @@ -161,7 +162,7 @@ Button.defaultProps = {
rel: null,
size: null,
target: null,
tooltipDelayDuration: 0,
tooltipDelayDuration: 'default',
variant: null
}

Expand All @@ -185,7 +186,7 @@ Button.propTypes = {
rel: PropTypes.string,
size: PropTypes.string,
target: PropTypes.string,
tooltipDelayDuration: PropTypes.number,
tooltipDelayDuration: PropTypes.oneOf(['default', 'slow']),
variant: PropTypes.string
}

Expand Down
5 changes: 2 additions & 3 deletions src/app/components/DownloadItem/DownloadItem.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,7 @@ const DownloadItem = ({
Icon: action.icon,
hideLabel: true,
variant: action.variant,
dataTestId: `download-item-${action.label}`,
tooltipDelayDuration: 300
dataTestId: `download-item-${action.label}`
}
}
/>
Expand Down Expand Up @@ -161,7 +160,7 @@ const DownloadItem = ({
<header className={styles.header}>
<Tooltip
content={itemName}
delayDuration={500}
duration="slow"
>
<h3
className={styles.name}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,14 @@ const DownloadListItemFileProgress = ({
<div className={styles.statusDescription}>
<p className={styles.statusInformation}>
{
state !== downloadStates.waitingForAuth && (
<FaFileDownload className={styles.statusInformationIcon} />
state !== downloadStates.waitingForAuth
&& state !== downloadStates.waitingForEula
&& (
<FaFileDownload
className={styles.statusInformationIcon}
role="graphics-symbol"
aria-label="A document with a downward facing arrow"
/>
)
}
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,7 @@ const DownloadListItemState = ({
state !== downloadStates.waitingForAuth
&& state !== downloadStates.waitingForEula
) && (
<Tooltip
content="View more info"
delayDuration={300}
>
<Tooltip content="View more info">
<Button
className={styles.hasErrorsButton}
align="baseline"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import '@testing-library/jest-dom'
import DownloadListItemFileProgress from '../DownloadListItemFileProgress'
import downloadStates from '../../../constants/downloadStates'

const fileIconLabel = 'A document with a downward facing arrow'

const setup = (overrideProps = {}) => {
const props = {
finishedFiles: 5,
Expand Down Expand Up @@ -37,6 +39,16 @@ describe('DownloadListItemFileProgress component', () => {

expect(screen.getByText('5 done in 3m, 54s (determining file count)')).toHaveClass('statusInformation')
})

test('shows the file icon', () => {
setup({
loadingMoreFiles: true
})

const fileIcon = screen.queryByLabelText(fileIconLabel)

expect(fileIcon).toBeInTheDocument()
})
})

describe('when shouldShowTime is false', () => {
Expand All @@ -47,6 +59,16 @@ describe('DownloadListItemFileProgress component', () => {

expect(screen.getByText('5 of 10')).toHaveClass('statusInformation')
})

test('shows the file icon', () => {
setup({
loadingMoreFiles: true
})

const fileIcon = screen.queryByLabelText(fileIconLabel)

expect(fileIcon).toBeInTheDocument()
})
})

describe('when shouldShowProgress is false', () => {
Expand All @@ -69,6 +91,18 @@ describe('DownloadListItemFileProgress component', () => {

expect(screen.getByText('Waiting for log in').parentElement).toHaveClass('statusInformation')
})

test('does not show the file icon', () => {
setup({
state: downloadStates.waitingForAuth,
shouldShowTime: false,
shouldShowProgress: false
})

const fileIcon = screen.queryByLabelText(fileIconLabel)

expect(fileIcon).not.toBeInTheDocument()
})
})

describe('when the state is waitingForEula', () => {
Expand All @@ -81,6 +115,18 @@ describe('DownloadListItemFileProgress component', () => {

expect(screen.getByText('Accept license agreement')).toHaveClass('statusInformation')
})

test('does not show the file icon', () => {
setup({
state: downloadStates.waitingForEula,
shouldShowTime: false,
shouldShowProgress: false
})

const fileIcon = screen.queryByLabelText(fileIconLabel)

expect(fileIcon).not.toBeInTheDocument()
})
})

describe('when the state is pending', () => {
Expand All @@ -91,6 +137,16 @@ describe('DownloadListItemFileProgress component', () => {

expect(container).toBeEmptyDOMElement()
})

test('does not show the file icon', () => {
setup({
state: downloadStates.pending
})

const fileIcon = screen.queryByLabelText(fileIconLabel)

expect(fileIcon).not.toBeInTheDocument()
})
})

describe('when the state is error', () => {
Expand All @@ -101,6 +157,16 @@ describe('DownloadListItemFileProgress component', () => {

expect(container).toBeEmptyDOMElement()
})

test('does not show the file icon', () => {
setup({
state: downloadStates.error
})

const fileIcon = screen.queryByLabelText(fileIconLabel)

expect(fileIcon).not.toBeInTheDocument()
})
})

describe('when the state is errorFetchingLinks', () => {
Expand All @@ -111,5 +177,15 @@ describe('DownloadListItemFileProgress component', () => {

expect(container).toBeEmptyDOMElement()
})

test('does not show the file icon', () => {
setup({
state: downloadStates.errorFetchingLinks
})

const fileIcon = screen.queryByLabelText(fileIconLabel)

expect(fileIcon).not.toBeInTheDocument()
})
})
})
1 change: 0 additions & 1 deletion src/app/components/Dropdown/Dropdown.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ const Dropdown = ({
Icon={FaEllipsisV}
hideLabel
tabIndex="0"
tooltipDelayDuration={300}
{...accessibleEventProps}
>
More Actions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@
&.spinner {
animation: spinnerRotate 2s linear infinite;
}
}
}
Loading

0 comments on commit 8c0b50b

Please sign in to comment.