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-22677: Better visuals for pagination #3881

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

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Feb 7, 2025

Jira URL

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

Changes

Description

  • Updated the DOM to fit the new design and improve accessibility of the pagination.
  • Separated the LivedataPagination in two parts so we can use it on both sides of the Topbar.
  • Updated tests.
  • Updated translations to fit the new design.
  • Updated CSS styles and layout.

Clarifications

  • To Be Written

Screenshots & Video

2025-02-07.17-59-28.mp4

This demo was made after the changes in 8439d9d . Note that I forgot to properly import the translation values on my local instance.

Executed Tests

Manual tests, see above.
Successfully built mvn clean install -f xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar and mvn clean install -f xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-macro.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • None, there's quite a lot of DOM changes, and at this point I don't think it'd be very safe to backport such an improvement on 16.10.X.

* Updated the DOM to fit the new design and improve accessibility of the pagination.
* Separated the LivedataPagination in two parts so we can use it on both sides of the Topbar.
* Updated tests.
* Updated translations to fit the new design.
* Updated CSS styles and layout.
@Sereza7
Copy link
Contributor Author

Sereza7 commented Feb 7, 2025

@tkrieck When you have time could you validate that the implementation I propose here is in line with your design and that I did not miss something important? Note that I only proposed an implementation for LiveData here, since we're supposed to move away from LT, I don't think it's worth the trouble to implement it for LT right now.

Thank you in advance!
Lucas C.

@tkrieck
Copy link
Contributor

tkrieck commented Feb 11, 2025

@Sereza7 Hi, thank you for taking on this proposal =) Your implementation seems ok to me, there are two small details though:

  1. The height of the selected page button seems to be a bit smaller than the proposal
  2. The select "results per page" is also smaller than the "more" button beside it, if possible it would be ideal to have both at the same size.

Both issues are very minor, mostly improvements, if they are too much of a hassle to implement don't worry about them.

Thank you very much and good job 👍

* Updated styles to better fit the design.
@Sereza7
Copy link
Contributor Author

Sereza7 commented Feb 11, 2025

  1. The height of the selected page button seems to be a bit smaller than the proposal

Okay, I just used the default sizing option from the btn-xs standard class, which imposes a minimum of 24*24 pxs. We can extend this to make it at least 30px high and give a more elongated look to the buttons.

  1. The select "results per page" is also smaller than the "more" button beside it, if possible it would be ideal to have both at the same size.

Changing it was having a large impact on the layout. But with the solution I ended up going with, it's kind of easy to have a good result. I was surprised that so few changes were enough to make this more consistent.


After taking your comments into account, I updated a bit the styles in 8b87d45 and here's what it looks like at a 100% zoom level:

image

@tkrieck
Copy link
Contributor

tkrieck commented Feb 11, 2025

After taking your comments into account, I updated a bit the styles in 8b87d45 and here's what it looks like at a 100% zoom level:

Awesome! All good on my side 👍

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