-
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
User group documentation #4106
User group documentation #4106
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4106 +/- ##
===========================================
+ Coverage 15.98% 16.01% +0.03%
===========================================
Files 732 734 +2
Lines 82054 82217 +163
Branches 1110 1119 +9
===========================================
+ Hits 13115 13167 +52
- Misses 68939 69050 +111
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.
Nice work Lanny! 🍨
Besides the inline comments,
-
I remember you wanting to use the latest screenshots with the latest avatar changes. Can we update the screenshots with those recent ones?
-
For later: It makes me think we should change the query in the Share Modal component and get the combined list of users/groups (first N) and show both user and group avatars.
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.
@lanzhenw this is looking good! Just left a few minor copy suggestions
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.
LGTM. Very nice! 🍨
What changes are proposed in this pull request?
Add documentation of user groups
How is this patch tested? If it is not, please explain why.
Screen.Recording.2024-02-28.at.10.09.54.AM.mov
(Details)
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyone
Python library changes