subcommunities: improve search page#1242
subcommunities: improve search page#1242alejandromumo wants to merge 8 commits intoinveniosoftware:masterfrom
Conversation
| @@ -0,0 +1,75 @@ | |||
| // This file is part of Invenio | |||
There was a problem hiding this comment.
This file is almost the same as https://github.com/inveniosoftware/invenio-requests/blob/master/invenio_requests/assets/semantic-ui/js/invenio_requests/search/RequestsResults.js (just renamed it). That makes me think this can be moved to a higher module so both requests and communities can reuse it.
There was a problem hiding this comment.
Maybe also in react-invenio-forms?
There was a problem hiding this comment.
this is a search aware component, we should move it to invenio-search-ui?
There was a problem hiding this comment.
this is a search aware component, we should move it to invenio-search-ui?
Exactly, I was just messaging Pablo about this. +1 on moving it to invenio-search-ui
There was a problem hiding this comment.
There was a problem hiding this comment.
The idea was to create a base package, similar to invenio-base for Python code, that serves as a catalog of components. This package should remain stateless, without any specific features or logic, making it a simple dependency that can be included in other modules.
While I understand the rationale behind associating a component that displays search results with invenio-search-ui, I believe that it doesn't align with the intended purpose of the base package.
The base package is meant to be a neutral catalog, and incorporating such a component into invenio-search-ui would go against these criteria.
I am sure that we will find a case where we will need that component in a place where we don't depend on invenio-search-ui.
Happy to hear/read counterarguments.
| <Menu.Item | ||
| as="button" | ||
| role="tab" | ||
| id="all-communities-tab" | ||
| aria-selected={selectedAppId === allCommunities.appId} | ||
| aria-controls={allCommunities.appId} | ||
| name="All" | ||
| active={selectedAppId === allCommunities.appId} | ||
| onClick={() => | ||
| this.setState({ | ||
| selectedConfig: allCommunities, | ||
| }) | ||
| } | ||
| > | ||
| {i18next.t("All")} | ||
| </Menu.Item> | ||
| <Menu.Item | ||
| as="button" | ||
| role="tab" | ||
| id="my-communities-tab" | ||
| aria-selected={selectedAppId === myCommunities.appId} | ||
| aria-controls={myCommunities.appId} |
There was a problem hiding this comment.
The front-page could reuse this "All / My communities" button. Is it worth moving it to its own component and re-using it? @ptamarit @0einstein0
I've found this to be a bit more complicated since it changes the apiConfig entirely
There was a problem hiding this comment.
I think that not showing the buttons to unauthenticated users (as currently done in the front page) would be better. It's more of a convenience than a crucial functionality; if a community owner happens to be logged out, they will still be able to find their community via text search.
There was a problem hiding this comment.
That's a good point. Regarding the question "should we make this a reusable component and use it in the frontpage", wdyt?
There was a problem hiding this comment.
Yes, right now, this is mostly the same as CommunitySelectionSearch in invenio-rdm-records.
It could go to react-invenio-forms and be used in all 3 places:
- The community front page
- The subcommunities search
- The deposit form community selection
There was a problem hiding this comment.
I don't think this component to filter between "All" or "My communities" should be in react-invenio-forms. It's too specific for communities. To be moved even higher in the modules, it had to be more abstract.
I added a reusable component CommunitiesStatusFilter in invenio_communities/community/searchComponents and used it in the 3 places you mentioned.
| // eslint-disable-next-line react/no-deprecated | ||
| ReactDOM.render( | ||
| <Container> | ||
| <CommunitySelectionSearch |
There was a problem hiding this comment.
I had to create a component due to the changes of config, based on the selection of "All" or "My communities". I could not connect the config with the component returned by createSearchAppInit, so I created a component that instantiates the search application.
...ts/semantic-ui/js/invenio_communities/community/searchComponents/CommunitySelectionSearch.js
Show resolved
Hide resolved
| export const overriddenComponents = { | ||
| [`BucketAggregation.element`]: ContribBucketAggregationElement, | ||
| [`BucketAggregationValues.element`]: ContribBucketAggregationValuesElement, | ||
| [`SearchApp.facets`]: ContribSearchAppFacetsWithConfig, | ||
| [`ResultsList.item`]: CommunityItem, | ||
| [`ResultsGrid.item`]: ResultsGridItemTemplate, | ||
| [`SearchBar.element`]: CommunitiesSearchBarElement, | ||
| [`SearchApp.results`]: SubcommunityResults, |
There was a problem hiding this comment.
Does it make sense to have the overridden components defined outside the element? Since this is now a "dedicated" search component for subcommunities, perhaps this could be defined in the component itself
There was a problem hiding this comment.
If we move the component to react-invenio-forms, we should use props rather than overrideable.
...ts/semantic-ui/js/invenio_communities/community/searchComponents/CommunitySelectionSearch.js
Outdated
Show resolved
Hide resolved
...ts/semantic-ui/js/invenio_communities/community/searchComponents/CommunitySelectionSearch.js
Outdated
Show resolved
Hide resolved
...ts/semantic-ui/js/invenio_communities/community/searchComponents/CommunitySelectionSearch.js
Outdated
Show resolved
Hide resolved
| <Menu.Item | ||
| as="button" | ||
| role="tab" | ||
| id="all-communities-tab" | ||
| aria-selected={selectedAppId === allCommunities.appId} | ||
| aria-controls={allCommunities.appId} | ||
| name="All" | ||
| active={selectedAppId === allCommunities.appId} | ||
| onClick={() => | ||
| this.setState({ | ||
| selectedConfig: allCommunities, | ||
| }) | ||
| } | ||
| > | ||
| {i18next.t("All")} | ||
| </Menu.Item> | ||
| <Menu.Item | ||
| as="button" | ||
| role="tab" | ||
| id="my-communities-tab" | ||
| aria-selected={selectedAppId === myCommunities.appId} | ||
| aria-controls={myCommunities.appId} |
There was a problem hiding this comment.
Yes, right now, this is mostly the same as CommunitySelectionSearch in invenio-rdm-records.
It could go to react-invenio-forms and be used in all 3 places:
- The community front page
- The subcommunities search
- The deposit form community selection
| @@ -0,0 +1,75 @@ | |||
| // This file is part of Invenio | |||
There was a problem hiding this comment.
Maybe also in react-invenio-forms?
invenio_communities/assets/semantic-ui/js/invenio_communities/subcommunity/index.js
Outdated
Show resolved
Hide resolved
| export const overriddenComponents = { | ||
| [`BucketAggregation.element`]: ContribBucketAggregationElement, | ||
| [`BucketAggregationValues.element`]: ContribBucketAggregationValuesElement, | ||
| [`SearchApp.facets`]: ContribSearchAppFacetsWithConfig, | ||
| [`ResultsList.item`]: CommunityItem, | ||
| [`ResultsGrid.item`]: ResultsGridItemTemplate, | ||
| [`SearchBar.element`]: CommunitiesSearchBarElement, | ||
| [`SearchApp.results`]: SubcommunityResults, |
There was a problem hiding this comment.
If we move the component to react-invenio-forms, we should use props rather than overrideable.
| <CommunitySelectionSearch | ||
| overriddenComponents={overriddenComponents} | ||
| config={config} | ||
| userAnonymous={userAnonymous} |
There was a problem hiding this comment.
For consistency, we can use myCommunitiesEnabled as done in zenodo/zenodo-rdm#1031
There was a problem hiding this comment.
I renamed myCommunitiesEnabled to communitiesStatusFilterEnabled since there's a reusable component CommunitiesStatusFilter to toggle between "All" and "My communities".
But followed the same pattern, looks good 😄

closes zenodo/zenodo-rdm#1015
Anonymous user

Authenticated user

** Smaller screens ªª
