-
Notifications
You must be signed in to change notification settings - Fork 594
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
Lightning UI #3807
Lightning UI #3807
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #3807 +/- ##
===========================================
+ Coverage 15.63% 15.85% +0.21%
===========================================
Files 672 720 +48
Lines 77832 79073 +1241
Branches 1041 1063 +22
===========================================
+ Hits 12168 12534 +366
- Misses 65664 66539 +875
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Great work! 🍨 nice refactor!
besides couple of nits, here is my suggestion
Major issue to investigate
The embeddings don't seem to work on my end. selecting a brain key has no effect.
Screen.Recording.2023-11-19.at.7.17.22.AM.mov
UX feedback - can come later
- There will be confusion for users who set the threshold, then try to filter using the sidebar and seeing "disabled" with colored downward carot without an indication as to why is it disabled and how can I reset the state so I can filter. One suggestion is to
- gray out the carot on the field that is disabled (keeping it colored is fine)
- show a button somewhere in the header so they can reset the threshold to none
- There are certain element(s) like Tagger that disappear in the lighning mode. could be confusing and raise lot of "Where is the Tagger?". My suggestion is to disable it and add a tooltip "Disabled - in lighning mode" or similar.
Question
app/packages/core/src/components/Filters/FilterOption/Selected.tsx
Outdated
Show resolved
Hide resolved
This is already the case. We can refine post dev cut 👍 |
Introduces new
FIFTYONE_APP_LIGHTNING_THRESHOLD
option that isNone
by default. It accepts a positive integer value. When a dataset's sample count exceeds that value and there is no view present, lightning mode is enabled in the App.In lightning mode, only indexed fields are filterable until the filtered count falls below the lightning threshold
Indexed filters filter have the following constraints
These constraints enable fast queries (IXSCAN and DISTINCT) via #3667
After filtering yields a sample count below the threshold, all other filters are accessible either through
see more
(for label fields with indexed subfields, or the top-level caret. Counts are also presentedChanges
I've added some hopefully improved organization to the
@fiftyone/core
and@fiftyone/state
packages to introduce new data flow whenlightning
mode is enabled.Notable code locations
@fiftyone/core
@fiftyone/state
Considerations I propose we include in separate PRs
see more
filtersTODO
Testing
Configure
Global wildcard index
Edges cases
Screen.Recording.2023-11-16.at.2.32.59.PM.mov