-
Notifications
You must be signed in to change notification settings - Fork 13
Pascal/pagination numbers fire #891
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
Conversation
Apparently this passed the "early slack design review", so pushing this for code review now. |
@@ -270,42 +271,6 @@ function SearchById() { | |||
); | |||
} | |||
|
|||
// Temporary disabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temporary since November, 2023. 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no such thing as temporarily commented code :D
We might want to also display the seconds (or change the formatting some way) if the first and last elements are within the same minute. It is quite likely that an organizaiton will have more than 25 (or even more than 50) decisions made within the same minutes, which would mean several pages would get the same pagination formatting, which in my opinion WouldBeBad.
|
You're right, will add tonight (or whenever I finish the PR) |
7c177a6
to
eb2a2e4
Compare
components={{ StartToEnd: <span style={{ fontWeight: 'bold' }} /> }} | ||
values={{ start, end }} | ||
/> | ||
{hideBoundaries ? null : endFormatted === null ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually hate nested ternaries, but js somehow pushes me to do it
I pushed commit 0e5a3035 with the following changes:
|
0e5a303
to
c3dd77c
Compare
add arabic translation
The last commit is a refactoring proposition, read it separately. |
aef406c
to
3726931
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Content
Part of preparatory work for checkmarble/marble-backend#1010.
It's a minimal downgrade of user experience, but will make things SO MUCH simpler on the backend side.
Sneak peak
What pagination without the rank numbers could look like:



first page
later page
if first and last element have created_at in the same month
NB: to discuss in which order to write it - on those screenshots it's in the order "from [first row] to [last row]", but that's opposite to temporal order with this pagination order.
While in the case manager it would be

because the order is not based only on the case dates (also on "boosted" status).
Disclaimer
As it is, it displays french format dates even though my selected locale is "english". This is not directly related to this PR, but is a side effect of a partial handling of language/format state in the app/browser, as explained in
packages/app-builder/src/utils/format.ts
.