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

plugin: sorting, filtering, selection fixes #146

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

takase1121
Copy link
Member

This ended up a bit larger than I think, so I would love to have another pair of eyes looking at this before getting it merged.

  1. You can now sort the table. Since the icon font is very limited, we don't even have proper icons to indicate the sort order. Currently I draw a line and I think this is less intuitive than it shold've been.
  2. You can also filter the table. This is different than "find" as it doesn't rely on the small CommandView preview section, and is generally a better UX. I think binding this as CTRL+F makes more sense than the current find-plugin, but I would like some more opinion on this.
  3. Arrow up and down now works correctly.

@takase1121
Copy link
Member Author

Screencast_20241130_212835.webm

More things to discuss:

  1. the sorting obviously isn't perfect. Version values aren't sorted correctly, and possible number values won't be sorted correctly as well since they're converted to a string.
  2. Refreshing and filtering does clear the selection. I assume this is already an issue since the object reference changes, but if there's a need to keep that consistent we will have to do ID checks.

@adamharrison
Copy link
Member

adamharrison commented Dec 2, 2024

  1. Yeah; the line isn't ideal, but it's better than nothing. I think it's good enough for now.
  2. I agree it should be used over find. It's better. We should probably just nix find entirely. There should be some indication we're filtering (statusview?), though; and escape should clear the filter.
  3. 👍
  4. I think it's fine for now that version numbers aren't sorted correctly, but we should support this.
  5. I think it's also OK to clear the selection; likely if you're filtering you don't really care about your selection anwyay.

@takase1121 takase1121 force-pushed the plugin/various-fixes branch from fea63c9 to 1221811 Compare December 2, 2024 07:04
@takase1121 takase1121 force-pushed the plugin/various-fixes branch from 1221811 to d269d5b Compare December 2, 2024 07:06
@takase1121
Copy link
Member Author

I made ctrl+f use filter, and implemented escape to clear filter text, and preserves filter text on subsequent ctrl+f.

As for the filtered status, I personally don't like putting messages that should be long running on the StatusView. Sure it is for that purpose but people easily miss it (for example, the find next press F3 thing). I instead put it on the top of the table.

Screencast_20241202_145120.webm

(Note: the offsetted entries are fixed in the latest commit)

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.

2 participants