Skip to content

Update sorting controls for Dandisets page #2358

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

Merged
merged 5 commits into from
Apr 30, 2025

Conversation

naglepuff
Copy link
Contributor

Fix #1764
Fix #2176

Changes

Replaces the current sorting controls on the Dandisets page with a more compact, mobile-friendly widget. Based on the design by @jtomeck outlined in #1764.

Demo video

compact_sort_widget_demo.mp4

@jjnesbitt
Copy link
Member

Should these controls not be on the left side, instead of the right? Since it's already a prominent change in the dandisets page, it might be easier for people to adjust if it's around where the old sort options were. cc @jtomeck

FWIW I really like the change.

@naglepuff
Copy link
Contributor Author

Should these controls not be on the left side, instead of the right? Since it's already a prominent change in the dandisets page, it might be easier for people to adjust if it's around where the old sort options were. cc @jtomeck

FWIW I really like the change.

That's a fair point. I don't necessarily have a preference, so I'm interested in seeing what others have to say. It's a straightforward adjustment to make if there's a consensus that it should be moved.

@naglepuff naglepuff requested a review from mvandenburgh April 28, 2025 18:04
Copy link
Member

@mvandenburgh mvandenburgh left a comment

Choose a reason for hiding this comment

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

With the latest code changes this look good to me. I just had one question about the design.

GitHub is also reporting a bunch of linting errors in the "Files changed" view for me, but CI is passing which is strange. I bet it's something wrong with how I set up linting for Vue 3. Either way, we can figure this out in a follow up since it's unrelated to this PR

@mvandenburgh mvandenburgh added the minor Increment the minor version when merged label Apr 28, 2025
@mvandenburgh
Copy link
Member

@naglepuff now that #2360 is merged, if you rebase this PR on master you should see those linting errors actually appear in CI

@naglepuff naglepuff force-pushed the 1764-make-sort-mobile-friendly branch from b2e7fee to a1016da Compare April 28, 2025 21:41
@naglepuff naglepuff force-pushed the 1764-make-sort-mobile-friendly branch from a1016da to cfdb5a6 Compare April 28, 2025 21:55
@naglepuff naglepuff requested a review from mvandenburgh April 29, 2025 17:59
@naglepuff
Copy link
Contributor Author

@mvandenburgh I see you just approved so sorry to make you take another look, but I worked on implementing the suggestions discussed earlier today

@jjnesbitt
Copy link
Member

jjnesbitt commented Apr 29, 2025

Maybe we should make "Sort By" and "Order" slightly bigger and/or bold? It's a bit hard to tell the two sections apart since those texts are so small/light.
image

Reference for how it is on github
image

Now that I'm looking at it, indentation is also very useful.

@naglepuff naglepuff force-pushed the 1764-make-sort-mobile-friendly branch from cfda90f to 01f1eae Compare April 29, 2025 18:18
@naglepuff
Copy link
Contributor Author

@jjnesbitt how about this?
image

@jjnesbitt
Copy link
Member

That looks great!

Copy link
Member

@mvandenburgh mvandenburgh left a comment

Choose a reason for hiding this comment

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

My bad, I approved this prematurely before - but agreed, this looks great!

@naglepuff naglepuff merged commit 9c420dd into master Apr 30, 2025
3 checks passed
@naglepuff naglepuff deleted the 1764-make-sort-mobile-friendly branch April 30, 2025 12:44
@dandibot
Copy link
Member

🚀 PR was released in v0.9.0 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

phone mode UI: no way to search on dandisets listing page Cant sort by size on mobile, button group bounces out of view
4 participants