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

Update pagination setting #81

Merged
merged 5 commits into from
Oct 25, 2023
Merged

Update pagination setting #81

merged 5 commits into from
Oct 25, 2023

Conversation

kunfang98927
Copy link
Contributor

  • remove the former setting button and modal
  • create a new pagination setting similar to Wikidata; three options - view 20/50/100 per page
  • add "Go to xxx Page"

@kunfang98927
Copy link
Contributor Author

I just updated the pagination setting. Maybe the review can be done after the project meeting so it's not rushed.

@dchiller
Copy link
Contributor

This looks great! I have only two comments:

image

I think it would be better to have "Go to Page XX" rather than "Go To XX Page". It's only European languages, so I'd be curious what other's find, but it looks better to me that way in English and when translated to Spanish and French than the way it currently is.

image

I wonder if the numbers here could either have more contrast or have some kind of divider ... they run together a bit for me here.

@kunfang98927
Copy link
Contributor Author

Thank you for the advice. I'm going to use 'Go to Page XXX'. Maybe I can add vertical lines between page numbers like Wikidata does.
image

@kunfang98927
Copy link
Contributor Author

Have updated the pagination setting. Now it looks like this
image

@dchiller
Copy link
Contributor

Looks great!

@@ -93,10 +94,9 @@ <h4 class="ms-3 me-2 my-auto"><small>INSTRUMENT LIST</small></h4>
</div>

<div class="d-flex">
<button type="button" class="btn me-1 px-1 py-0 justify-content-center display-btn" id="page-setting-btn" data-bs-toggle="modal" data-bs-target="#paginationModal">
{% comment %} <button type="button" class="btn me-1 px-1 py-0 justify-content-center display-btn" id="page-setting-btn" data-bs-toggle="modal" data-bs-target="#paginationModal">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to keep this comment in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we don't need the setting button at present, I'll delete it.

@kunfang98927 kunfang98927 merged commit c753820 into develop Oct 25, 2023
1 check passed
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