-
Notifications
You must be signed in to change notification settings - Fork 75
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
feat: refined ux update a taxonomy by downloading and uploading [FC-0036] #732
Merged
xitij2000
merged 98 commits into
openedx:master
from
open-craft:rpenido/fal-3539-refined-ux-update-a-taxonomy-by-downloading-and-uploading
Jan 16, 2024
Merged
Changes from all commits
Commits
Show all changes
98 commits
Select commit
Hold shift + click to select a range
0c60c37
feat: add import taxonomy feature
rpenido c273af1
test: add api.test.js
rpenido 047530d
test: add import button click test
rpenido 303e7ba
test: add action.test.js
rpenido 3955e91
test: add more tests to action.tests.js
rpenido 8176973
test: add more tests to action.tests.js 2
rpenido 4f31714
Merge branch 'openedx:master' into rpenido/fal-3532-import-taxonomy
rpenido df441fb
fix: import
rpenido a0e92f0
test: simplify import test
rpenido dc8ed9e
fix: remove undefined var
rpenido 120154e
refactor: rename actions.js -> utils.js
rpenido 318bbb7
revert: change in the jest.config
rpenido a8c2fd5
Merge branch 'master' into rpenido/fal-3532-import-taxonomy
pomegranited 70ac2a7
Merge branch 'openedx:master' into rpenido/fal-3532-import-taxonomy
rpenido adacf23
chore: trigger CD/CI
rpenido 5983953
Merge branch 'master' into rpenido/fal-3532-import-taxonomy
rpenido 7756c7f
feat: import tags to existing taxonomy
rpenido 4202c27
Merge branch 'master' into rpenido/fal-3532-import-taxonomy
rpenido 6bcd924
refactor: merges TaxonomyCardMenu and TaxonomyDetailMenu (#13)
rpenido f318dfc
refactor: improve menu organization
rpenido 4c22a0a
feat: refined ux update a taxonomy by downloading and uploading
rpenido 63a1487
fix: eslint
rpenido 1110979
fix: design adjustments
rpenido 8eac9cb
fix: more design adjustments
rpenido 830c991
test: fix tests
rpenido 7a0a44c
test: fix api test
rpenido 6a55c6f
refactor: move getFileSizeToClosestByte to root utils
rpenido 1119041
fix: invalidate taxonomyDetail query
rpenido f56a05d
test: add testing-library/react-hooks and fix testing
rpenido cb52fd7
fix: rename files
rpenido 94f1bf7
test: fix api test
rpenido b63147a
test: check invalidateQueries
rpenido 3613f22
test: improve LoadingButton tests
rpenido 2b8d534
test: fix LoadingButton test
rpenido 895bbc4
test: add ImportTagsWizard tests
rpenido 538d003
test: more ImportTagsWizard tests
rpenido 1eea5d2
fix: types
rpenido d91716b
feat: add typings from code review
rpenido 244b83b
refactor: typings and return of taxonomy import api
rpenido c85ebdc
fix: type import
rpenido 204b2b9
fix: types
rpenido 707b1bd
Merge branch 'rpenido/fal-3532-import-taxonomy' into rpenido/fal-3539…
rpenido 5e0f599
Merge branch 'master' into rpenido/fal-3532-import-taxonomy
rpenido f888ecf
refactor: merging conflicts
rpenido 3c8da66
refactor: change export syntax
rpenido 40479c1
fix: eslint
rpenido 46e5d1b
fix: export TaxonomyDetail
rpenido db8ba58
fix: eslint
rpenido 8fe8c07
Merge branch 'openedx:master' into rpenido/fal-3532-import-taxonomy
rpenido aa5fc4f
Merge branch 'rpenido/fal-3532-import-taxonomy' into rpenido/fal-3539…
rpenido 9fce34b
fix: TaxonomyContext types
rpenido 1440215
fix: TaxonomyContext types
rpenido 89f8116
feat: add toast and error message
rpenido 5dd25a2
fix: layout alert message
rpenido 676a636
fix: types and lint
rpenido 23c28af
fix: lint
rpenido ad69e11
fix: error message
rpenido f6cfcf3
Merge branch 'rpenido/fal-3532-import-taxonomy' into rpenido/fal-3539…
rpenido 2d3abe8
fix: use scss utility
rpenido cfa01b6
style: add PR in comment
rpenido e5b8e58
Merge branch 'master' into rpenido/fal-3532-import-taxonomy
rpenido fafcb9b
chore: rebasing
rpenido e80194f
fix: types
rpenido e0f1a40
Merge branch 'rpenido/fal-3532-import-taxonomy' into rpenido/fal-3539…
rpenido 8bcbb64
fix: missing }
rpenido 1e0dde3
fix: remove typo
rpenido 7cae57b
test: cleaning test removing useMutation mock
rpenido 1002fc6
style: cleaning code and fix lint
rpenido a33aa8a
Merge branch 'rpenido/fal-3532-import-taxonomy' into rpenido/fal-3539…
rpenido 7900876
fix: import taxonomy type
rpenido 46b7b3e
revert: changes in package-lock.json
rpenido e54fd45
refactor: rename close -> onClose
rpenido a51bb40
test: change to getByText
rpenido fdc74c7
Merge branch 'master' into rpenido/fal-3532-import-taxonomy
rpenido 9a35f1a
Merge branch 'rpenido/fal-3532-import-taxonomy' into rpenido/fal-3539…
rpenido ef20d55
fix: add a div over the dialog to prevent the user to interact while …
rpenido 65ca5a9
fix: add title to dialog to remove console warning
rpenido ff234b4
fix: add title property to Stepper.Step to fix warning
rpenido 72d82d6
fix: remove warning when toastMessage = null
rpenido 3e5931d
Merge branch 'master' into rpenido/fal-3539-refined-ux-update-a-taxon…
rpenido 67e0f7b
fix: merging errors
rpenido a6e1a8f
fix: typo
rpenido 9321f4e
test: change .toThrow => .not.toBeInDocument
rpenido e427e82
fix: reverting some minor changes
rpenido 2e81a5c
fix: remove console warnings
rpenido ae9657d
chore: trigger CI
rpenido a86f597
refactor: getFileSizeToClosestByte
rpenido 71b025e
test: remove waitFor and change e.click() to fireEvent.click(e)
rpenido 2637165
fix: remove disabled from dialog
rpenido fd897b3
Merge branch 'master' into rpenido/fal-3539-refined-ux-update-a-taxon…
rpenido 7400aa4
fix: remove old test file
rpenido bbb0b19
refactor: LoadingButton
rpenido e82a49c
fix: alertProps type
rpenido 4e42254
style: fix eslint
rpenido 971de66
fix: remove wrong comment
rpenido 2c05a81
test: fix LoadingButton tests
rpenido f0af395
fix: add catch to LoadingButton click to fix test
rpenido dd3e972
fix: change requested modular-learning#176
rpenido File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,10 @@ | |
color: $black; | ||
} | ||
|
||
.h-200px { | ||
height: 200px; | ||
} | ||
|
||
.mw-300px { | ||
max-width: 300px; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
import React from 'react'; | ||
import { | ||
act, | ||
fireEvent, | ||
render, | ||
} from '@testing-library/react'; | ||
|
||
import LoadingButton from '.'; | ||
|
||
const buttonTitle = 'Button Title'; | ||
|
||
const RootWrapper = (onClick) => ( | ||
<LoadingButton label={buttonTitle} onClick={onClick} /> | ||
); | ||
|
||
describe('<LoadingButton />', () => { | ||
it('renders the title and doesnt handle the spinner initially', () => { | ||
const { container, getByText } = render(RootWrapper(() => { })); | ||
expect(getByText(buttonTitle)).toBeInTheDocument(); | ||
|
||
expect(container.getElementsByClassName('icon-spin').length).toBe(0); | ||
}); | ||
|
||
it('doesnt render the spinner without onClick function', () => { | ||
const { container, getByRole, getByText } = render(RootWrapper()); | ||
const titleElement = getByText(buttonTitle); | ||
expect(titleElement).toBeInTheDocument(); | ||
expect(container.getElementsByClassName('icon-spin').length).toBe(0); | ||
fireEvent.click(getByRole('button')); | ||
expect(container.getElementsByClassName('icon-spin').length).toBe(0); | ||
}); | ||
|
||
it('renders the spinner correctly', async () => { | ||
let resolver; | ||
const longFunction = () => new Promise((resolve) => { | ||
resolver = resolve; | ||
}); | ||
const { container, getByRole, getByText } = render(RootWrapper(longFunction)); | ||
const buttonElement = getByRole('button'); | ||
fireEvent.click(buttonElement); | ||
expect(container.getElementsByClassName('icon-spin').length).toBe(1); | ||
expect(getByText(buttonTitle)).toBeInTheDocument(); | ||
// StatefulButton only sets aria-disabled (not disabled) when the state is pending | ||
// expect(buttonElement).toBeDisabled(); | ||
expect(buttonElement).toHaveAttribute('aria-disabled', 'true'); | ||
|
||
await act(async () => { resolver(); }); | ||
|
||
expect(buttonElement).not.toHaveAttribute('aria-disabled', 'true'); | ||
expect(container.getElementsByClassName('icon-spin').length).toBe(0); | ||
}); | ||
|
||
it('renders the spinner correctly even with error', async () => { | ||
let rejecter; | ||
const longFunction = () => new Promise((_resolve, reject) => { | ||
rejecter = reject; | ||
}); | ||
const { container, getByRole, getByText } = render(RootWrapper(longFunction)); | ||
const buttonElement = getByRole('button'); | ||
|
||
fireEvent.click(buttonElement); | ||
|
||
expect(container.getElementsByClassName('icon-spin').length).toBe(1); | ||
expect(getByText(buttonTitle)).toBeInTheDocument(); | ||
// StatefulButton only sets aria-disabled (not disabled) when the state is pending | ||
// expect(buttonElement).toBeDisabled(); | ||
expect(buttonElement).toHaveAttribute('aria-disabled', 'true'); | ||
|
||
await act(async () => { rejecter(new Error('error')); }); | ||
|
||
// StatefulButton only sets aria-disabled (not disabled) when the state is pending | ||
// expect(buttonElement).toBeEnabled(); | ||
expect(buttonElement).not.toHaveAttribute('aria-disabled', 'true'); | ||
expect(container.getElementsByClassName('icon-spin').length).toBe(0); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
// @ts-check | ||
import React, { | ||
useCallback, | ||
useEffect, | ||
useRef, | ||
useState, | ||
} from 'react'; | ||
import { | ||
StatefulButton, | ||
} from '@edx/paragon'; | ||
import PropTypes from 'prop-types'; | ||
|
||
/** | ||
* A button that shows a loading spinner when clicked. | ||
* @param {object} props | ||
* @param {string} props.label | ||
* @param {function=} props.onClick | ||
* @param {boolean=} props.disabled | ||
* @returns {JSX.Element} | ||
*/ | ||
const LoadingButton = ({ | ||
label, | ||
onClick, | ||
disabled, | ||
}) => { | ||
const [state, setState] = useState(''); | ||
// This is used to prevent setting the isLoading state after the component has been unmounted. | ||
const componentMounted = useRef(true); | ||
|
||
useEffect(() => () => { | ||
componentMounted.current = false; | ||
}, []); | ||
|
||
const loadingOnClick = useCallback(async (e) => { | ||
if (!onClick) { | ||
return; | ||
} | ||
|
||
setState('pending'); | ||
try { | ||
await onClick(e); | ||
} catch (err) { | ||
// Do nothing | ||
} finally { | ||
if (componentMounted.current) { | ||
setState(''); | ||
} | ||
} | ||
}, [componentMounted, onClick]); | ||
|
||
return ( | ||
<StatefulButton | ||
disabledStates={disabled ? [state] : ['pending'] /* StatefulButton doesn't support disabled prop */} | ||
onClick={loadingOnClick} | ||
labels={{ default: label }} | ||
state={state} | ||
/> | ||
); | ||
}; | ||
|
||
LoadingButton.propTypes = { | ||
label: PropTypes.string.isRequired, | ||
onClick: PropTypes.func, | ||
disabled: PropTypes.bool, | ||
}; | ||
|
||
LoadingButton.defaultProps = { | ||
onClick: undefined, | ||
disabled: undefined, | ||
}; | ||
|
||
export default LoadingButton; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of button.click etc, you should ideally use fireEvent.click(button) or userEvent.click(button)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: 71b025e