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

GrafanaUI: Add two icons add-user and clip-attach #87112

Merged
merged 2 commits into from May 1, 2024
Merged

Conversation

reemtariqq
Copy link
Contributor

@reemtariqq reemtariqq commented Apr 30, 2024

What is this feature?

Add two icons to the uniconsadd-userand clip-attach
for more : Slack thread

Screenshot 2024-04-30 at 10 53 11 Screenshot 2024-04-30 at 10 53 05

Special notes for your reviewer:

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.

@reemtariqq reemtariqq requested review from a team as code owners April 30, 2024 08:55
@reemtariqq reemtariqq requested review from joshhunt, eledobleefe and jackw and removed request for a team April 30, 2024 08:55
@grafana-delivery-bot grafana-delivery-bot bot added this to the 11.1.x milestone Apr 30, 2024
@@ -245,6 +245,8 @@ export const availableIconsIndex = {
'wrap-text': true,
rss: true,
x: true,
'add-user': true,
'clip-attach': true,
Copy link
Contributor

Choose a reason for hiding this comment

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

i think paperclip is a better name, but 🤷

Choose a reason for hiding this comment

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

I see you @joshhunt. Same thought about the "add-user" but it is named from the Unicon official library. 🤷🏻‍♂️

Copy link

Choose a reason for hiding this comment

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

hey folks 👋🏼 actually I just checked and the names of these icons in Unicon is add-account and attach. I think the names we are putting forward are the best ones for discoverability, but feel free if you'd like to rename - no big deal.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel strongly about this (hence why I approved - you can merge whenever you're ready), but either paperclip or attach seem like the best options to me. add-user is fine to me.

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, i renamed it to be attach

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshhunt i'm not one of the authorized users to merge the PR, would you please ?

@joshhunt joshhunt changed the title add two icons add-user and clip-attach GrafanaUI: Add two icons add-user and clip-attach Apr 30, 2024
@reemtariqq reemtariqq added the no-changelog Skip including change in changelog/release notes label Apr 30, 2024
@joshhunt joshhunt merged commit fcb40e6 into main May 1, 2024
18 checks passed
@joshhunt joshhunt deleted the add-user-icon branch May 1, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend no-changelog Skip including change in changelog/release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants