Panel Header Fix: Implement new Panel Header on Angular Panels #66826

axelavargas · 2023-04-19T09:48:57Z

What is this feature?

Use the new panel header on angular panels.

Research effort required to use PanelChrome on PanelChromeAngular
Implement PanelChrome on angular panels

Idea from Torkel: refactor out a bit of code from PanelStateWrapper (something like getPanelChromeProps(panel, plugin, data)) so we can share some logic between PanelStateWrapper and PanelChromeAngular

Use helper function getPanelChromeProps on PanelStateWrapper
Make sure e2e and unit tests pass
Make sure the new flow is not impacting the e2e test from plugin examples https://github.com/grafana/grafana-plugin-examples (If yes, fix it)

Why do we need this feature?

Prevent a mix of UI on dashboards that have angular and react panels

Who is this feature for?

everyone

Which issue(s) does this PR fix?:

Fixes #66282

Special notes for your reviewer:

Should I target this change to milestone v10? or 9.5.x?

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.

angular panel chrome

grafanabot · 2023-04-19T13:08:16Z

Hello @axelavargas!
Backport pull requests need to be either:

Pull requests which address bugs,
Urgent fixes which need product approval, in order to get merged,
Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

dprokop

@axelavargas tested this with http://localhost:3000/d/pttable/panel-tests-table?orgId=1 and http://localhost:3000/d/5SdHCadmz/panel-tests-graph?orgId=1 I couldn't find any issues so far.

public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx

dprokop · 2023-04-20T10:19:06Z

public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx

+          data-testid={selectors.components.Panels.Panel.title(panel.title)}
+          aria-label={selectors.components.Panels.Panel.containerByTitle(panel.title)}


Wonder if lack of these when using the new PanelChrome wouldn't cause some test to fail?

Yes, in the past we got some failed tests when we enabled the newPanelChromeUI feature flag by default, not only because of these selectors but also because we changed the flow of opening a menu. Marcus helped us in implementing code to maintain backward compatibility between the old angular panels and new panels PR. But, I think this code is not needed anymore as we will have the same flow for angular panels and react panels.

torkelo

This looks really good but looks like you can move a lot more of the props to getPanelChromeProps

public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx

public/app/features/dashboard/utils/getPanelChromeProps.ts

public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx

polibb · 2023-04-20T11:08:12Z

I can see some issues related to updating the panel when changing its settings: Change of the DS, description, links, transparency, etc. do not change the data or look of the panel until after the dashboard is saved and the page reloaded.

Screen.Recording.2023-04-20.at.14.01.24.mov

Changing the transparency, for example, is effective "immediately" if you change the panel type to a non-angular one and go back to the angular one:

Video: From angular panel to react panel, transparency udpate

Screen.Recording.2023-04-20.at.13.56.22.mov

NB! However, I can also see those problems on Ops right now:

Video: reproducing in ops

Screen.Recording.2023-04-20.at.13.59.20.mov

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

axelavargas · 2023-04-24T12:51:01Z

I can see some issues related to updating the panel when changing its settings: Change of the DS, description, links, transparency, etc. do not change the data or look of the panel until after the dashboard is saved and the page reloaded.

Thank you @polibb 🙌🏾, I've tested locally and confirmed that these are indeed bugs. As you mentioned, they also exist in the current implementation (I tested them in ops). For now, I won't address them in this PR since they are out of scope. We can discuss this problem with the team and evaluate whether we should fix these issues, keeping in mind that Angular panels are deprecated.

PanelStateWrapper

axelavargas · 2023-04-25T11:12:58Z

@torkelo I applied all the suggestions 😄
✔️ Returned title items directly
✔️ Moved drag class, title, and description to getPanelChromeProps
✔️ Extracted common props from PanelChromeAngular and PanelStateWrapper

torkelo · 2023-04-25T11:21:03Z

public/app/features/dashboard/utils/getPanelChromeProps.tsx

+import { PanelPadding } from '@grafana/ui';
+import { InspectTab } from 'app/features/inspector/types';
+import { getPanelLinksSupplier } from 'app/features/panel/panellinks/linkSuppliers';
+import { plugin } from 'app/plugins/panel/alertGroups/module';


@axelavargas I discovered a bug, the panel padding is not working / correctly set for panels that have the "noPadding" option set (Stat panel, old angular table panel).

The problem is that your importing plugin from here instead of accessing it from props :)

oh! will fix it :)

polibb

Manually tested with CarpetPlot and d3 - LGTM! Beautiful!

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com> (cherry picked from commit 2a67b8a)

…ls (#67228) Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com> Co-authored-by: Alexa V <239999+axelavargas@users.noreply.github.com> Fix: Implement new Panel Header on Angular Panels (#66826)

gtk-grafana · 2023-05-01T16:13:40Z

https://ops.grafana-ops.net/d/eef643/mimir-reads?orgId=1&refresh=10s

https://github.com/grafana/grafana/blob/main/public/app/features/dashboard/utils/getPanelChromeProps.tsx#L44

I get nearly 100 of these dashboards_panelheader_description_displayed rudderstack events just scrolling to the bottom of a big dashboard, and they just keep coming. Do we really need to make this many requests? Can we aggregate and send these events out in batches or something if this data collection is critical?

@axelavargas

axelavargas · 2023-05-22T10:22:24Z

Hey @gtk-grafana, thank you so much for pointing out this issue 🙌🏾 . Unfortunately, I overlooked the message 😢 and only became aware of the problem when it was brought up again today in our Slack channel. I have opened a PR to address this, as it isn't the expected behavior.

axelavargas added 2 commits April 18, 2023 17:22

Use PanelChrome inside PanelAngularChrome , duplicate functions for now

de16ef6

Implement helper function to get panel chrome props and use it inside

b3c0fb6

angular panel chrome

axelavargas added the area/frontend label Apr 19, 2023

axelavargas added this to the 10.0.0 milestone Apr 19, 2023

axelavargas self-assigned this Apr 19, 2023

axelavargas requested review from torkelo, mckn, a team, evictorero, kaydelaney and dprokop and removed request for a team April 19, 2023 09:49

axelavargas modified the milestones: 10.0.0, 9.5.x Apr 19, 2023

axelavargas added add to changelog backport v9.5.x Bot will automatically open backport PR labels Apr 19, 2023

grafanabot added the missing-labels label Apr 19, 2023

axelavargas added the type/bug label Apr 19, 2023

grafanabot removed the missing-labels label Apr 19, 2023

dprokop reviewed Apr 20, 2023

View reviewed changes

torkelo reviewed Apr 20, 2023

View reviewed changes

public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx Outdated Show resolved Hide resolved

public/app/features/dashboard/utils/getPanelChromeProps.ts Outdated Show resolved Hide resolved

public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx Outdated Show resolved Hide resolved

Update public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx

fdc5619

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

axelavargas added 3 commits April 24, 2023 16:55

Return description from panel chrome props

aab60dc

Refactor: Extract commom props in getPanelChromeProps and use it on

6df5fb4

PanelStateWrapper

Move title and drag class to getPanelChromeProps

80f3291

axelavargas requested review from torkelo and dprokop April 25, 2023 09:32

axelavargas marked this pull request as ready for review April 25, 2023 09:32

axelavargas requested a review from a team as a code owner April 25, 2023 09:32

Refactor: return title items directly

37490de

torkelo reviewed Apr 25, 2023

View reviewed changes

axelavargas added 2 commits April 25, 2023 13:21

remove leftover function

ef56bba

Fix plugin.noPadding not being taken from props

af01619

axelavargas requested a review from torkelo April 25, 2023 11:32

torkelo approved these changes Apr 25, 2023

View reviewed changes

polibb approved these changes Apr 25, 2023

View reviewed changes

Merge branch 'main' into axelav/migrate-to-angular-panels

a47469f

axelavargas changed the title ~~Panel Header: Implement new Panel Header on Angular Panels~~ Panel Header Fix: Implement new Panel Header on Angular Panels Apr 25, 2023

axelavargas merged commit 2a67b8a into main Apr 25, 2023

axelavargas deleted the axelav/migrate-to-angular-panels branch April 25, 2023 16:16

grafanabot mentioned this pull request Apr 25, 2023

[v9.5.x] Panel Header Fix: Implement new Panel Header on Angular Panels #67228

Merged

grafanabot pushed a commit that referenced this pull request Apr 25, 2023

Panel Header Fix: Implement new Panel Header on Angular Panels (#66826)

4347536

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com> (cherry picked from commit 2a67b8a)

zerok modified the milestones: 9.5.x, 9.5.2, 10.0.0 May 3, 2023

axelavargas mentioned this pull request May 22, 2023

Dashboard: Fix panel description event triggering every time panel is rendered #68791

Merged

3 tasks

zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Panel Header Fix: Implement new Panel Header on Angular Panels #66826

Panel Header Fix: Implement new Panel Header on Angular Panels #66826

axelavargas commented Apr 19, 2023 •

edited

Loading

grafanabot commented Apr 19, 2023

dprokop left a comment

dprokop Apr 20, 2023

axelavargas Apr 24, 2023

torkelo left a comment

polibb commented Apr 20, 2023 •

edited by axelavargas

Loading

axelavargas commented Apr 24, 2023

axelavargas commented Apr 25, 2023

torkelo Apr 25, 2023

axelavargas Apr 25, 2023

polibb left a comment

gtk-grafana commented May 1, 2023 •

edited

Loading

axelavargas commented May 22, 2023

		data-testid={selectors.components.Panels.Panel.title(panel.title)}
		aria-label={selectors.components.Panels.Panel.containerByTitle(panel.title)}

Panel Header Fix: Implement new Panel Header on Angular Panels #66826

Panel Header Fix: Implement new Panel Header on Angular Panels #66826

Conversation

axelavargas commented Apr 19, 2023 • edited Loading

grafanabot commented Apr 19, 2023

dprokop left a comment

Choose a reason for hiding this comment

dprokop Apr 20, 2023

Choose a reason for hiding this comment

axelavargas Apr 24, 2023

Choose a reason for hiding this comment

torkelo left a comment

Choose a reason for hiding this comment

polibb commented Apr 20, 2023 • edited by axelavargas Loading

axelavargas commented Apr 24, 2023

axelavargas commented Apr 25, 2023

torkelo Apr 25, 2023

Choose a reason for hiding this comment

axelavargas Apr 25, 2023

Choose a reason for hiding this comment

polibb left a comment

Choose a reason for hiding this comment

gtk-grafana commented May 1, 2023 • edited Loading

axelavargas commented May 22, 2023

axelavargas commented Apr 19, 2023 •

edited

Loading

polibb commented Apr 20, 2023 •

edited by axelavargas

Loading

gtk-grafana commented May 1, 2023 •

edited

Loading