-
Notifications
You must be signed in to change notification settings - Fork 881
fix(site): display not found page when pagination page is invalid #12611
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
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.
@BrunoQuaresma Can you implement this accordingly to what we agreed with @mafredri or is it impossible now?
In my fantasy, we'd show a tooltip/banner that this page doesn't exist, but we'd still show the page 85 button as selected, with the ability to select a previous page (that exists).
(post)
@mtojek can you be more specific? In this case, what is missing is the pagination widget right? |
Yes, I can. The reason why we implemented the What I can see here is just "Go to the first page", but the idea is to render the last available page. |
Go to the last page would add one redirect and maybe unexpected IMO. What I see other apps doing is a not found error message, wdyt @mafredri ? |
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.
one small suggestion for code readability, but I like the look of this! thanks for the change!
Here's how I think about it @BrunoQuaresma. Imagine the following situation:
As a user, I would like for the pagination to contain This way, I can easily recover and choose whether or not I want to go to the first or the last page (1 or 2). Given more pages it could look like I think encountering a 404 in my described scenario would feel pretty weird and unexpected (since it used to he a page). |
@mafredri I see that, I just think redirecting the user to the last page is not usual, at least in other apps, but I'm ok with showing the pagination. |
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.
👍
Closes #11795