feat: display all child tags in the "bare bones" taxonomy detail page [FAL-3529] [FC-0036] #10
Conversation
|
@pomegranited @rpenido FYI - I don't want to delay openedx#655 so I've made this as a separate PR, but this is an improvement I want to make before we update our sandbox and get users testing it. |
|
Hi @bradenmacdonald! LGTM. Just need to fix some lint issues to make the tests pass. If running without openedx/openedx-learning#119 there is some weird behaviors: Edit:
Running the openedx/openedx-learning#119 branch fixed it. |
There was a problem hiding this comment.
LGTM 👍
- I read through the code
- I followed the test instructions and ran the code locally
- I checked for accessibility issues by navigating using the keyboard only
- User-facing strings are extracted for translation
@bradenmacdonald
Do you think it is better to merge in the branch opencraft branch now or wait openedx#655 get merged and then create a new PR against master?
src/generic/Loading.jsx
Outdated
| @@ -2,27 +2,30 @@ import React from 'react'; | |||
| import { Spinner } from '@edx/paragon'; | |||
| import { FormattedMessage } from '@edx/frontend-platform/i18n'; | |||
|
|
|||
|
|
|||
| import { useSubTags } from './data/api'; | ||
|
|
||
| const SubTagsExpanded = ({ taxonomyId, parentTagValue }) => { | ||
|
|
|
|
||
| if (subTagsData.isLoading) { | ||
| return <LoadingSpinner />; | ||
| } else if (subTagsData.isError) { |
There was a problem hiding this comment.
lint
| } else if (subTagsData.isError) { | |
| } | |
| if (subTagsData.isError) { |
|
@rpenido Thanks! I'll wait until your PR is merged (as well as openedx/openedx-learning#119 ), and then I'll re-submit this PR. I don't want to slow down your PR from getting merged. |
* feat: System-defined tooltip added * feat: Taxonomy card menu added. Export menu item added * feat: Modal for export taxonomy * feat: Connect with export API * test: Tests for API and selectors * feat: Use windows.location.href to call the export endpoint * test: ExportModal.test added * style: Delete unnecesary code * docs: README updated with taxonomy feature * style: TaxonomyCard updated to a better code style * style: injectIntl replaced by useIntl on taxonomy pages and components * refactor: Move and rename taxonomy UI components to match 0002 ADR * refactor: Move api to data to match with 0002 ADR * test: Refactor ExportModal tests * chore: Fix validations * chore: Lint * refactor: Moving hooks to apiHooks * feat: add taxonomy detail page * fix: address nits in PR review * refactor: move data/selectors to data/apiHooks and fix tests to mock useQuery. * fix: address nits in PR review * fix: replace taxonomy menu ModalPopup with Dropdown menu Avoids clicking through to the card when using the menu button to hide a card's menu. * fix: change taxonomy URLs * /taxonomy-list is now /taxonomies, and there's a temporary redirect * /taxonomy-list/:id: is now /taxonomy/:id: --------- Co-authored-by: Christofer <[email protected]> Co-authored-by: XnpioChV <[email protected]> Co-authored-by: Christofer Chavez <[email protected]> Co-authored-by: Jillian Vogel <[email protected]> Co-authored-by: Braden MacDonald <[email protected]>
bradenmacdonald
force-pushed
the
braden/improve-listing-tags-bare-bones
branch
from
c3d9f73 to
9f4d892
Compare
|
@rpenido Now that your original PR has merged, I have a new PR. Can you please review it there? openedx#703 |
This is a minor improvement to openedx#655
It sort of depends on openedx/openedx-learning#119 but it can be merged even before that one is merged.
This is not the final UI design and all of this can/will be changed - this just makes sure that users can see all the tags in the taxonomy during our initial round of testing.
Before:
After: