-
Notifications
You must be signed in to change notification settings - Fork 3
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
New faceted search results component #132
Conversation
as the upgraded version (1.3.0) was erred with webpack loader error TODO: fix this later
…/atlas-components into feature/new_faceted_search_component
Randomise species, cell types and organism parts
…onse structure from backend
TODO: improve the test, clean the code
WIP for the other groups
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.
Some style issues and a few opportunities to dry out code. Almost there!
packages/scxa-faceted-search-results-v5/src/facetgroups/CheckboxFacetGroup.js
Outdated
Show resolved
Hide resolved
packages/scxa-faceted-search-results-v5/src/facetgroups/CheckboxFacetGroup.js
Outdated
Show resolved
Hide resolved
packages/scxa-faceted-search-results-v5/src/facetgroups/CheckboxFacetGroupsDefaultProps.js
Outdated
Show resolved
Hide resolved
packages/scxa-faceted-search-results-v5/cypress/component/MultiselectDropdownFacetGroup.cy.js
Outdated
Show resolved
Hide resolved
packages/scxa-faceted-search-results-v5/cypress/component/FilterSidebar.cy.js
Outdated
Show resolved
Hide resolved
packages/scxa-faceted-search-results-v5/cypress/component/FilterList.cy.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Alfonso Muñoz-Pomer Fuentes <[email protected]>
Dropdown menus populate properly but I notice a few points to fix or to improve. Please take a look below.
|
Replying to @lingyun1010 's latest comment on this PR:
We earlier discussed how to refresh each filers in the filter side bar with Nancy and I implemented that way.
Thanks for spotting this. I fixed it. I did it differently than in the original implementation to self contain the tooltip component. It looks cleaner.
Thanks for spotting this. I removed all the sorting related code from the demo page as we decided not to have sorting at all in the new version. We are going to add it if the users require it for some reason.
This is not a bug in the UI. The UI only shows the values that it gets from the backend. It is the backend's responsibility to filter out any invalid value if it needs to be. I am going to discuss it with the data prod team. |
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.
Good stuff! Two minor corrections and all good.
packages/scxa-faceted-search-results-v5/src/facetgroups/FacetGroupPropTypes.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Alfonso Muñoz-Pomer Fuentes <[email protected]>
Co-authored-by: Alfonso Muñoz-Pomer Fuentes <[email protected]>
This new component is going to replace the current
scxa-faceted-search-result
atlas component.The current search component loaded all the results into memory and did the search result modification on that sometimes huge data structure. That was not ideal and sometimes we even got a timeout error when the time to return the response was longer than the allowed timeout value of our firewall.
In this change we are using separate endpoints for each of the filter components that was created in our backend previously:
Any changes in the filters triggers to reload/repopulate the values of the other filters and the search result.
I also did a smaller optimisation for limiting the number of REST calls: the currently modified filter component is not going to call its endpoint to reload its value as it is not necessary, because we have them already.
On the backend we are about to start a ticket to make the search result paginated. When that is ready we should make sure that this frontend component could perfectly can communicate with that modified endpoint and can handle the search result, too.
Suggestion for improvement in following the PRs: