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

Improved pagination design in file browser #1311

Merged
merged 4 commits into from
Nov 21, 2022

Conversation

mvandenburgh
Copy link
Member

Fixes #725.

I implemented Jared's design from #725, and also added an error message when an invalid path is entered by the user.
Previously, a user would only run into this error if they manually edited the query params in the browser URL bar, so our very basic solution was to just kick the user back to the root directory of the dandiset if that happened. But now that there's an input to specify the page number, it's much easier for a user to enter something invalid. So, I've changed this behavior to display an error message instead of kicking them back to the root.
I still think the UX here could use some significant improvement, but for the purposes of this PR I think it's sufficient.

@mvandenburgh
Copy link
Member Author

This seems to be a good staging dandiset to try this out on https://deploy-preview-1311--gui-staging-dandiarchive-org.netlify.app/dandiset/201029/draft/files?location=

@satra
Copy link
Member

satra commented Oct 4, 2022

it would be nice to know how many pages there are.

@mvandenburgh mvandenburgh marked this pull request as draft October 4, 2022 23:19
@mvandenburgh mvandenburgh removed the request for review from jjnesbitt October 4, 2022 23:19
@waxlamp
Copy link
Member

waxlamp commented Oct 10, 2022

it would be nice to know how many pages there are.

@mvandenburgh, is it known on each render how many total files/pages there are? If yes, it should be straightforward to add an of <N> after the combobox.

@mvandenburgh
Copy link
Member Author

it would be nice to know how many pages there are.

@mvandenburgh, is it known on each render how many total files/pages there are? If yes, it should be straightforward to add an of <N> after the combobox.

Yes, I can add that. Sorry, I thought I replied to @satra's comment but I guess I didn't submit it.

@mvandenburgh mvandenburgh force-pushed the file-browser-pagination-redesign branch 2 times, most recently from 4085bc1 to ea0a41a Compare October 21, 2022 20:11
@mvandenburgh mvandenburgh marked this pull request as ready for review October 21, 2022 20:12
@jjnesbitt
Copy link
Member

Maybe when the user inputs an invalid page, it could error but without making the failing call to the API (so just staying on the last valid page)? Currently, if they input something invalid, the page immediately shrinks, and it's more awkward to use.

Copy link
Member

@waxlamp waxlamp left a comment

Choose a reason for hiding this comment

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

Changes look good. I think there's a very subtle issue with saying "no items found at the specified path" for a non-existent folder (it should be a "harsher" error message to distinguish between a folder with no items and a folder that doesn't exist--even though the latter doesn't actually happen in DANDI) but for now I think this is fine.

@waxlamp
Copy link
Member

waxlamp commented Oct 27, 2022

Changes look good. I think there's a very subtle issue with saying "no items found at the specified path" for a non-existent folder (it should be a "harsher" error message to distinguish between a folder with no items and a folder that doesn't exist--even though the latter doesn't actually happen in DANDI) but for now I think this is fine.

I forgot to mention: I think Jake's comment is a good idea; you could implement that now or defer to later--I'll leave that between you and Jake.

@mvandenburgh
Copy link
Member Author

mvandenburgh commented Oct 27, 2022

Maybe when the user inputs an invalid page, it could error but without making the failing call to the API (so just staying on the last valid page)? Currently, if they input something invalid, the page immediately shrinks, and it's more awkward to use.

@AlmightyYakob Implemented this 7626033 (#1311)

@mvandenburgh
Copy link
Member Author

mvandenburgh commented Nov 3, 2022

Coming back to this, I noticed a bug with the "error" page - marking this as draft until I fix that fixed

@mvandenburgh mvandenburgh removed the request for review from jjnesbitt November 3, 2022 17:58
@mvandenburgh mvandenburgh marked this pull request as draft November 3, 2022 17:58
@mvandenburgh mvandenburgh force-pushed the file-browser-pagination-redesign branch from 7626033 to 8f3e75c Compare November 21, 2022 15:26
@mvandenburgh mvandenburgh marked this pull request as ready for review November 21, 2022 15:31
@mvandenburgh mvandenburgh added the enhancement New feature or request label Nov 21, 2022
@mvandenburgh mvandenburgh merged commit c60ef30 into master Nov 21, 2022
@mvandenburgh mvandenburgh deleted the file-browser-pagination-redesign branch November 21, 2022 15:48
@dandibot
Copy link
Member

🚀 PR was released in v0.3.2 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reduce pagination thumbs
5 participants