ReactSearchKit: pass initial query state [DEPENDS ON inveniosoftware/react-searchkit#137] #214
Conversation
There was a problem hiding this comment.
I'm afraid it only addresses the issue partly; for instance consider clicking on one of the three button present on the frontsite homepage. Moreover, this was not mentioned in the original issue but it seems all the search forms are affected by this problem (backoffice in particular).
|
I fixed it for the other searches as well except the document search in the frontsite series details. |
| @@ -40,6 +40,8 @@ class Sidebar extends Component { | |||
| const checkInActive = activePath.includes(BackOfficeRoutes.checkIn); | |||
| const checkOutActive = activePath.includes(BackOfficeRoutes.checkOut); | |||
|
|
|||
| const query = '&sort=mostrecent&p=1'; | |||
There was a problem hiding this comment.
the issue is that it might be that we have some searches that do not support the mostrecent sorting. Do we really need this?
| @@ -51,7 +51,7 @@ const HeadlineLayout = props => { | |||
| className="headline-quick-access" | |||
| as={Link} | |||
| to={FrontSiteRoutes.documentsListWithQuery( | |||
| '&sort=mostrecent&order=desc' | |||
| '&sort=mostrecent&order=desc&p=1' | |||
There was a problem hiding this comment.
I think ReactSearchKit has a feature to add default values when not provided, so I am not sure what is wrong here. We need to investigate RSK in my opinion.
There was a problem hiding this comment.
I would probably go for that too, since the issue can still be reproduced if provided a url without the p=1 parameter; ie. https://localhost:3000/search?q=. Thanks Konstantina for working on it, it's a very annoying issue that has been here for a long time.
|
I think the issue is in RSK, this one: inveniosoftware/react-searchkit#110 |
KonstantinaStoikou
changed the title
KonstantinaStoikou
changed the title
There was a problem hiding this comment.
For the sorting filters it's much more coherent now, good job. Here is some additional context to the issue (Karolina can correct me):
- By default a sorting filter is a pair of parameters: the field to compare (title, date) and a traversal order (asc, desc)
- For the frontsite we want this feature to me more user-friendly, therefore instead of having one input for each parameter we provide a unified one which carries both informations at the same time (ie. "Title [A-Z]"). It allows to have an explicit name for the sorting filter and we can also get rid of useless filters that do not belong to any reasonable use case (ie. "Best match + Desc" can be interpreted as "Worst match" which is not very useful...)
- For the backoffice we would like to allow more flexibility and independence, reason why there are two inputs. However it seems that some search pages have this input while others don't; I think we should stick to the same thing everywhere otherwise I find it confusing
Could you fix this typo by the way? Thanks
| defaultValueOnEmptyString={searchConfig.SORT_BY_ON_EMPTY_QUERY} | ||
| overridableId="mobile" | ||
| /> | ||
| <SortBy values={searchConfig.SORT_BY} overridableId="mobile" /> |
There was a problem hiding this comment.
I think that the SortBy/SortOrder should completely disappear, in favor of Sort only as in the frontiste. WDYT?
Yes exactly: @kprzerwa started to move from It is much easier, IMHO, to use on cmp as you have described then 2. |
|
The major fixes were successfully re-tested with dependent PR inveniosoftware/react-searchkit#137. This PR is now waiting for Edit: done. |
FlorianCassayre
changed the title
* update react-searchkit to 1.0.0-alpha.4 * fix frontsite series cover flickering * pagination failing when overflowing from the allowed ES window * removed the last asc/desc inputs in the backoffice
Closes #196
Removes sortOrder dropdown from searches (and merges it with sortBy dropdown) (except for searches that have many date options, e.g. Loans)
Closes #207