Closed Ducica closed 1 month ago
ntarocco
commented
2 months ago It makes sense to make this behaviour configurable: as a generic principle, we tend to avoid hiding components from the UI so that users can comprehend what is happening. This use case though makes a lot of sense. If you need this soon, would you be available to create a PR?
Ducica
commented
2 months ago @ntarocco Hi Nicola, it is not some critical issue for us, but I would be OK to make a PR for it. It should probably be during next week. I will let you know when it is done and then I assume you would give me permissions to push to this repo and also let me know the person I should put as a reviewer. Have a nice evening.
Ducica
commented
2 months ago @ntarocco Hi Nicola, I've prepared a small PR for this subject. Could you please allow me to push to the repo as well as the name of the reviewer. Thanks.
ntarocco
commented
2 months ago @ntarocco Hi Nicola, I've prepared a small PR for this subject. Could you please allow me to push to the repo as well as the name of the reviewer. Thanks.
Thank you, please fork this repo and create a PR from the fork.
Ducica
commented
2 months ago Hello, here it is https://github.com/inveniosoftware/react-searchkit/pull/268. Please take a look, I've never followed this procedure before, so I hope I did not make any mistakes.
@ntarocco PS, I am not sure if you automatically get notified even though I did not tag you explicitly. If yes, please ignore and sorry for the spam. Also, if I did not do something according to how you do things, let me know and I would be happy to fix it. It is my first time contributing to Invenio software, so I hope it is forgivable :0 Have a nice weekend
ntarocco
commented
1 month ago Ducica
commented
1 month ago @ntarocco You are welcome Nicola. If it would be OK for you, I would also solve
https://github.com/inveniosoftware/react-invenio-forms/issues/236
and
https://github.com/inveniosoftware/react-invenio-forms/issues/230
Which are causing us issues. Especially the accordion issue, as our users would like to have some accordions folded when the form first loads and it is just not possible to do at the moment. I am thinking this could be done in one PR. If you agree, let me know please, I would probably get to it by end of this week.
The pagination component as well as results per page, should have a bit different condition for being rendered in my opinion. I.e. they should only be rendered when totalResults>currentSize && currentSize>-1 (and not as currently currentSize > -1 && totalResults > 0).
When we get very few results, we have a situation where the pagination with one page only just sits there, takes up space and frankly looks weird (same goes for results per page i.e. it is unnecessary when totalResults<currentSize). Currently, you need to wrap it with your own component that uses withState and then conditionally render as suggested above, which just bloats the code for no reason and is not very convenient to do in some cases and looks like it would be best handled on the level of component itself.