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

Grafana/ui: Add UserIcon and UsersIndicator components #66906

Merged
merged 20 commits into from May 25, 2023

Conversation

Clarity-89
Copy link
Contributor

What is this feature?

Move the UserIcon and UsersIndicator components into grafana/ui.

Why do we need this feature?

Enable this component to be used by other parts of Grafana + plugins.

Which issue(s) does this PR fix?:

Part of #66679.

Special notes for your reviewer:

Default state:
Screenshot 2023-04-19 at 13 06 02
Screenshot 2023-04-19 at 14 21 38

Active state:
Screenshot 2023-04-20 at 8 49 02

With many users
Screenshot 2023-04-20 at 8 50 00

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@Clarity-89 Clarity-89 added this to the 10.0.0 milestone Apr 20, 2023
@Clarity-89 Clarity-89 self-assigned this Apr 20, 2023
@Clarity-89 Clarity-89 requested review from a team as code owners April 20, 2023 06:12
@Clarity-89 Clarity-89 requested review from jackw and tskarhed and removed request for a team April 20, 2023 06:12
@Clarity-89 Clarity-89 changed the title grafana/ui: Add UserIcon and UsersIndicator components Grafana/ui: Add UserIcon and UsersIndicator components Apr 20, 2023
Comment on lines +33 to +44
* Output the initials of the first and last name (if given), capitalized and concatenated together.
* If name is not provided, an empty string is returned.
* @param {string} [name] The name to extract initials from.
* @returns {string} The uppercase initials of the first and last name.
* @example
* // Returns 'JD'
* getUserInitials('John Doe');
* // Returns 'A'
* getUserInitials('Alice');
* // Returns ''
* getUserInitials();
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I love this!

Copy link
Contributor

@JoaoSilvaGrafana JoaoSilvaGrafana left a comment

Choose a reason for hiding this comment

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

Nice job on moving these 👍
just a couple of comments

}

const limitReached = users.length > limit;
const extraUsers = users.length - limit;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a check here in case the limit is negative? It doesn't make any sense but you can set it in storybook to -1, it ends up with a confusing result because then if there are 5 users the extraUsers are 6 so it displays 4 user icons and then +6 🙃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Also added min param to the argType but it looks like it doesn't apply to manually typed values.

Copy link
Contributor

@JoaoSilvaGrafana JoaoSilvaGrafana left a comment

Choose a reason for hiding this comment

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

Nice, lgtm! 👏

@Clarity-89 Clarity-89 merged commit 41b609d into main May 25, 2023
15 checks passed
@Clarity-89 Clarity-89 deleted the grafana-ui/user-icon branch May 25, 2023 12:57
@zerok zerok modified the milestones: 10.0.0, 10.0.0-preview, 10.1.x May 31, 2023
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants