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

Show complete rows on pages by default. #168

Merged
merged 3 commits into from
Jan 17, 2023
Merged

Conversation

joaander
Copy link
Member

Description

Set PER_PAGE to 24.

Motivation and Context

With the default cards per row of 3, this makes the default view show complete rows to the end of the page. Prior to this change, the last row is left incomplete because 25 is not an integer multiple of 3.

Checklist:

With the default cards per row of 3, this makes the default view
show complete rows to the end of the page.
@joaander joaander requested review from a team as code owners December 19, 2022 19:50
@joaander joaander requested review from vyasr and tommy-waltmann and removed request for a team December 19, 2022 19:50
@codecov
Copy link

codecov bot commented Dec 19, 2022

Codecov Report

Merging #168 (85cede9) into master (7c7a696) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 85cede9 differs from pull request most recent head 5882493. Consider uploading reports for the commit 5882493 to get more accurate results

@@           Coverage Diff           @@
##           master     #168   +/-   ##
=======================================
  Coverage   73.32%   73.32%           
=======================================
  Files          20       20           
  Lines         791      791           
=======================================
  Hits          580      580           
  Misses        211      211           
Impacted Files Coverage Δ
signac_dashboard/dashboard.py 72.75% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

changelog.txt Outdated Show resolved Hide resolved
@bdice
Copy link
Member

bdice commented Dec 20, 2022

The original motivation for 25 was that a number around 25 felt like a reasonable size for the page, and I liked that 4 pages = 100 jobs. I don't have a strong feeling about this and if users find it compelling to have 24 instead, that is fine.

@joaander
Copy link
Member Author

Users can change the setting. I just though others might find the default behavior annoying:
Screen Shot 2022-12-21 at 7 57 28 AM
So much white space!

@bdice bdice merged commit be2ca23 into master Jan 17, 2023
@bdice bdice deleted the update-per-page-default branch January 17, 2023 16:55
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.

3 participants