-
Notifications
You must be signed in to change notification settings - Fork 0
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: use search manager (TEMP) #44
feat: use search manager (TEMP) #44
Conversation
When lib mode is set to "mixed", both "Libraries" and "Legacy Libraries" tabs are show in the Studio Home. When "Libraries" is clicked, v2 libraries are fetched, when "Legacy Libraries" is clicked, v1 libraries are fetched. When lib mode is set to "v1 only" or "v2 only", only one tab "Libraries" is show and only the respective libraries are fetched when the tab is clicked.
This is to switch between different library modes.
The path updates when selecting tabs, when accessing the url with the path directly it will open its respective tab. Navigating using the browser back/forward buttons is also supported.
This commit is temporary as the current frontend build system in tests doesnt support TS syntax. That should be fixed soon, and this commit should be removed.
This is a temporary commit since there are currently no webpack loaders that support tsx files in the test running. This commit should be removed once that is fixed upstream.
When lib mode is set to "mixed", both "Libraries" and "Legacy Libraries" tabs are show in the Studio Home. When "Libraries" is clicked, v2 libraries are fetched, when "Legacy Libraries" is clicked, v1 libraries are fetched. When lib mode is set to "v1 only" or "v2 only", only one tab "Libraries" is show and only the respective libraries are fetched when the tab is clicked.
This is to switch between different library modes.
The path updates when selecting tabs, when accessing the url with the path directly it will open its respective tab. Navigating using the browser back/forward buttons is also supported.
This commit is temporary as the current frontend build system in tests doesnt support TS syntax. That should be fixed soon, and this commit should be removed.
This is a temporary commit since there are currently no webpack loaders that support tsx files in the test running. This commit should be removed once that is fixed upstream.
…753-library-home-page-bare-bones
b948788
to
f902335
Compare
…/fal-3764-library-home-filter-by-content-type
cf0e3c1
to
1bf2708
Compare
a418e39
to
07b3a03
Compare
07b3a03
to
dfd3190
Compare
78bbf84
to
3400f71
Compare
showContent: resultShowContent, | ||
}; | ||
}, [isFetching, isFetchingNextPage]); | ||
const showLoading = isFetching || isFetchingNextPage; |
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.
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.
I think it looks much nicer without the loading, nice addition!
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.
@rpenido 👍 Great work on this! I've tested it out and everything is looking good, just left a new few nits/comments.
- I tested this:
- Tested filtering by tags
- Tested searching for tags to filter by
- Tested filtering by block type
- Tested clearing all applied filters
- Tested clearing a single applied filter
- Tested searching still works as expected
- I read through the code
- I checked for accessibility issues
-
Includes documentation
src/search-manager/FilterByTags.tsx
Outdated
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.
The labels in the Tags filter dropdown are not aligned:
We can add the mb-0
class to this line in this file in order to fix the issue.
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.
Fixed here: 1718c26
<Tab eventKey={TAB_LIST.home} title="Home" /> | ||
<Tab eventKey={TAB_LIST.components} title="Components" /> | ||
<Tab eventKey={TAB_LIST.collections} title="Collections" /> |
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.
nit: The titles should be translated (this should probably be changed in the base branch cc: @ChrisChV)
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.
showContent: resultShowContent, | ||
}; | ||
}, [isFetching, isFetchingNextPage]); | ||
const showLoading = isFetching || isFetchingNextPage; |
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.
I think it looks much nicer without the loading, nice addition!
2672082
to
c453ef0
Compare
…nido/fal-3764-library-home-filter-by-content-type
0a962a0
to
32838a3
Compare
Closed in favor of: openedx#1141 |
No description provided.