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

Library Panels: Add library panel tab to share modal #32953

Merged
merged 2 commits into from Apr 15, 2021
Merged

Conversation

kaydelaney
Copy link
Contributor

What this PR does / why we need it:
Adds the ability to add a panel to the panel library through a new tab on the panel share modal.

Which issue(s) this PR fixes:
Relates #31305

@kaydelaney kaydelaney added this to the 8.0.0 milestone Apr 13, 2021
@kaydelaney kaydelaney self-assigned this Apr 13, 2021
@kaydelaney kaydelaney requested a review from a team April 13, 2021 15:37
@kaydelaney kaydelaney added this to In Review in Frontend Platform Backlog via automation Apr 13, 2021
@kaydelaney kaydelaney requested review from mckn, oscarkilhed and hugohaggmark and removed request for a team April 13, 2021 15:37
Copy link
Contributor

@hugohaggmark hugohaggmark left a comment

Choose a reason for hiding this comment

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

Looks great and works as expected 🎉


export const ShareGlobalPanel = ({ panel, initialFolderId, onDismiss }: Props) => {
if (!panel) {
return <></>;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Maybe return null here?

Comment on lines +11 to +14
type ShareModalTabPropsWithInternalModel = ShareModalTabProps & { panel?: InternalPanelModel };
export type ShareModalTab =
| React.ComponentType<ShareModalTabProps>
| React.ComponentType<ShareModalTabPropsWithInternalModel>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I wish we didn't need to reference the InternalPanelModel here, but I can see that would become hard to fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a bit strange actually, looking at some of the other Share modal components (ShareSnapshot, ShareEmbed, and ShareExport) all use the internal PanelModel for their panel prop, while the ShareModalTab type seems to use the @grafana/data PanelModel interface. Not exactly sure why typescript didn't complain about that, but did for the component I've introduced with this PR, but I suspect it has something to do with the fact that the component I've introduced is a function component whereas the others are all class components.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine let's get this merged.

@hugohaggmark hugohaggmark mentioned this pull request Apr 14, 2021
31 tasks
@kaydelaney kaydelaney merged commit 2ca4f3e into master Apr 15, 2021
Frontend Platform Backlog automation moved this from In Review to Done Apr 15, 2021
@kaydelaney kaydelaney deleted the lp-share-modal branch April 15, 2021 07:29
davidmparrott added a commit that referenced this pull request Apr 15, 2021
* set query in rules response

* Theme: tweaking dark theme colors (#33007)

* Library Panels: Add library panel tab to share modal (#32953)

* Explore: Scroll split panes in Explore independently (#32978)

* Change default prometheus to latest and prometheus v1 to prometheus1

* Update README

* Remove prometheus1 block as not used

* Explore: Separatae scrolling in split view

* Update snapshot

* Allow skip migrations in tests via environment variable (#32958)

* Dashboard: Fix issue where Slack notifications won't link to users (#32861)

* DashboardPage: refactored styles from sass to emotion (#32955)

* DashboardPage: refactored styles from sass to emotion

* refactored dashboardPage component to be alot easier to read and understand

* more refactoring...

* more cleaning...

* fixes frontend test

* fixes frontend test- I hope

* fixes frontend test- I hope

* moves dashboard scss styles back to it's standalone file

* GraphNG: use theme font family and size for axis labels (#33009)

* GraphNG: use theme font family and size for axis labels

* fix test

* AlertingNG: Slack notification channel (#32675)

* AlertingNG: Slack notification channel

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Add tests

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix review comments

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix review comments and small refactoring

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* GraphNG: stacking (#30749)

* First iteration

* Dev dash

* Re-use StackingMode type

* Fix ts and api issues

* Stacking work resurected

* Fix overrides

* Correct values in tooltip and updated test dashboard

* Update dev dashboard

* Apply correct bands for stacking

* Merge fix

* Update snapshot

* Revert go.sum

* Handle null values correctyl and make filleBelowTo and stacking mutual exclusive

* Snapshots update

* Graph->Time series stacking migration

* Review comments

* Indicate overrides in StandardEditorContext

* Change stacking UI editor, migrate stacking to object option

* Small refactor, fix for hiding series and dev dashboard

* VizLegend: sets a min and max value of the seriesCount control in Storybook (#33022)

* Alerting: Filter rules list (#32818)

* Chore: Reduces strict errors (#33012)

* Chore: reduces strict error in OptionPicker tests

* Chore: reduces strict errors in FormDropdownCtrl

* Chore: reduces has no initializer and is not definitely assigned in the constructor errors

* Chore: reduces has no initializer and is not definitely assigned in the constructor errors

* Chore: lowers strict count limit

* Tests: updates snapshots

* Tests: updates snapshots

* Chore: updates after PR comments

* Refactor: removes throw and changes signature for DashboardSrv.getCurrent

* [Alerting]: Several modifications in alert rules (#32983)

* [Alerting]: Use common properties for all rules

* Add Labels in rules

* Fix update ruleGroup API

Return 400 Bad Request response
when the request contains a UID that does not exist

* Check permissions and return namespace id

* Apply suggestions from code review

Co-authored-by: gotjosh <josue@grafana.com>

* WIP (#33025)

* Chore: Bump strict error count limit (#33035)

* set query in rules response

Co-authored-by: Torkel Ödegaard <torkel@grafana.org>
Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com>
Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com>
Co-authored-by: Dafydd <72009875+dafydd-t@users.noreply.github.com>
Co-authored-by: n-wbrown <n-wbrown@users.noreply.github.com>
Co-authored-by: Uchechukwu Obasi <obasiuche62@gmail.com>
Co-authored-by: Leon Sorokin <leeoniya@gmail.com>
Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com>
Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
Co-authored-by: Nathan Rodman <nathanrodman@gmail.com>
Co-authored-by: Hugo Häggmark <hugo.haggmark@grafana.com>
Co-authored-by: Sofia Papagiannaki <papagian@users.noreply.github.com>
Co-authored-by: gotjosh <josue@grafana.com>
Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
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

3 participants