Skip to content
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

XWIKI-21910: Display pagination of a LiveData only if number of entries exceed 10 #3030

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Mar 28, 2024

Jira URL

https://jira.xwiki.org/browse/XWIKI-21910

Changes

Description

  • Updated both layouts to not display the pagination tools if there's only one page of entries

Clarifications

  • Note that I went with the solution of hiding the pagination if the initial state of the livedata has only one page. IMO this is easier to understand for users than a cutoff at numberOfEntries=10. When merging this, I'll update the Livedata doc to make sure users can find out what's happening if they couldn't fitgure it out on their own.

Screenshots & Video

Here are what a few default flavor livedatas look like after the changes in this PR:
Menu listing
21910-afterPRMenu
Notification preferences filters
21910-afterPRNotificationPreferences

Executed Tests

Successfully built mvn clean install -f xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar -Pdocker,integration-tests,quality. Made sure to not add warnings to the npm run lint.
I doubt another module integration tests using a livedata would fail since this pagination element is only removed in case it's useless.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • None, master only.

…es exceed 10

* Updated both layouts to not display the pagination tools if there's only one page of entries
@manuelleduc
Copy link
Contributor

  • Note that I went with the solution of hiding the pagination if the initial state of the livedata has only one page. IMO this is easier to understand for users than a cutoff at numberOfEntries=10. When merging this, I'll update the Livedata doc to make sure users can find out what's happening if they couldn't fitgure it out on their own.

Imo it shouldn't be the default behavior. Or at least it should be configurable, i.e., provide an option to allow the pagination to be hidden when there is only one page.
I'd suggest making a proposal.

@Sereza7 Sereza7 marked this pull request as draft March 28, 2024 15:11
@Sereza7
Copy link
Contributor Author

Sereza7 commented May 28, 2024

Started a proposal to discuss this topic at https://forum.xwiki.org/t/deactivate-livedata-pagination-when-theres-only-one-page/14669 .

…es exceed 10

* Removed the changes from the livedata layouts
* Updated the macro parameters
* Added logic on the pagination element to use this parameter.
* Updated the spec.js files to go with that

TODO:
* Check that I didn't forget anything.
* Run locally and test the feature
* Run automatic tests
(* Create a new automatic test for the feature.)
…es exceed 10

* Changed the position of the property in the metadata to be more coherent with others
* Added the default value of the configuration and the macro parameter initialization.
* Fixed automated tests
* Ran locally and video recorded the successful tests
@Sereza7
Copy link
Contributor Author

Sereza7 commented Nov 5, 2024

In regards to the results of the proposal, I updated the implementation:

Changes

Description

  • Added a showPaginationOnSinglePage parameter to the Livedata macro.

Screenshots & Video

I ran the changes locally and got the following results:

2024-11-05.10-23-25.mp4

We can see on this video that the parameter changes the display of the livedata in the expected way. Note that there's one edge case where this might be confusing for the user. When we don't want to show the pagination on a single page and there's initially multiple pages. If we increase the size of the pages to have only one, the livedata will be reloaded and the pagination will not be available anymore. We can address this possible confusion with a small note in the documentation, IMO this is better to stay consistent here and not introduce extra logic. Moreover, I expect this parameter to be used mostly on livedatas with a known number of elements that fills up only one page or less.

Executed Tests

Successfully built mvn clean install -f xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-api; mvn clean install -f xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar. Ran it locally with expected results on multiple livedata configurations (see manual test video above).

@Sereza7 Sereza7 marked this pull request as ready for review November 5, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants