-
Notifications
You must be signed in to change notification settings - Fork 12.2k
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
Connections console: Add Angular badge for Angular plugins #70789
Connections console: Add Angular badge for Angular plugins #70789
Conversation
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.
Left a comment about the condition. Not only a matter of preference but safer code. see https://codesandbox.io/s/autumn-sunset-dp7vdj?file=/src/App.js
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
@torkelo Yeah it does look a bit off. I have only added the "Angular" badge in this PR. I have tried moving the new badge below the logo, added a bit more padding and made the logo bigger to make it more similar to the plugins list: It still look a bit empty compared to the plugins list, but I think it's better than what we have now. WDYT? Shall I push this version instead? |
I pushed a branch here (https://github.com/grafana/grafana/compare/connections-card-design) with some changes to how to correctly extend/modify Card component to add things like badges, I also aligned the styling with the data source cards (used in new data source page), cards that looks so much better. I think with description and the [Core] Badge (or Installed) to also mark which data sources are bundled plugins (or installed) would also be useful. And this list really needs some default sorting to show show the popular ones first (or popular in a separate group). With description (did not add the [Core] or [Installed] badges, as it requires more changes to the component props to pass in that info |
Thank you, I will merge it with this branch 🙌 |
…useppe/angular-deprecation/connections-angular-badge
I have merged those changes into this PR's code. I decided to keep the description hidden for now, since it messes up the spacing if there's too much text, and the purpose of this PR was just to add the Angular badge. e.g.: The other changes can be done in another PR if that's okay, I sent the feedback to connections working group, thanks :) |
@xnyo yea, let's skip description for now. Needs some fine tuning to skip / cut long descriptions |
And the card grid template needs update, it positions the description strangly |
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! Great suggestions by @torkelo
About showing the description on the cards: |
* Angular deprecation: Add Angular badge in plugin catalog page * Angular deprecation: Add alert in plugin details page * Angular deprecation: Disable install button if for Angular plugins * removed extra console.log * Add tests for Angular badge * Add tests for PluginDetailsAngularDeprecation * Add tests for InstallControlsButton * Add tests for ExternallyManagedButton * Table tests * Catalog: Update angular deprecation message * PR review feedback * Update tests * Update copy for angular tooltip and alert * Update tests * Fix test warnings * Fix angularDetected not being set for remote catalog plugins * Dynamic alert text based on grafana config * Connections: Add Angular badge to Angular plugins * Add test for connections console angular badge * Fix tests * Fix tests * Moved tests * PR review: Use ternary operator instead of && * Fixes to how to use Card component * comment out desc * Do not use deprecated theme.v1 and theme.typography.size * pr review feedback --------- Co-authored-by: Torkel Ödegaard <torkel@grafana.com>
What is this feature?
Adds "Angular" badge next to Angular plugins to the on-prem connections console.
Why do we need this feature?
Angular deprecation
Who is this feature for?
Grafana users who rely on Angular datasources
Which issue(s) does this PR fix?:
Related to #68974
Special notes for your reviewer:
Please check that: