-
Notifications
You must be signed in to change notification settings - Fork 12k
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
Adds empty list CTA and confirm delete button to Api Keys Page #13471 #13608
Conversation
@mondras thanks for your contribution. Please sign the Contributor License Agreement (CLA). If you've tried to sign, but still didn't work you've probably used a different email address in git commits than you have specified in Github. You should be able to add that other email to Github and then you should be able to sign the CLA. |
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 @mondras. Thank you!
a299683
to
dc9e822
Compare
Please sign CLA |
@@ -7,6 +7,7 @@ const model = { | |||
buttonIcon: 'ga css class', | |||
buttonLink: 'http://url/to/destination', | |||
buttonTitle: 'Click me', | |||
onClick: 'handler', |
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.
What's the reason for p[assing a string into the onClick prop? Wouldn't it make more sense to use jest.fn()
?
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.
Good point, will update it.
@mondras thanks for your contribution. Please sign the Contributor License Agreement (CLA). If you've tried to sign, but still didn't work you've probably used a different email address in git commits than you have specified in Github. You should be able to add that other email to Github and then you should be able to sign the CLA. |
@mondras can you sign the CLA so we can merge this? |
Done, sorry about the delay. Also updated tests based on @peterholmberg 's input. |
Fixes #13471
Wasn't sure if a new route
org/apikeys/new
with its controller was needed.Also had to add an
onClick
prop toEmptyListCTA
to show the hidden form.Thanks Grafana team! We use your tool on a daily basis, it was time to give some back.
Screenshots of how it looks:
Empty CTA page:
Adding keys on CTA page:
Confirm deletion: