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

Scene: Wrap Drawer component for scene #87347

Closed
wants to merge 1 commit into from

Conversation

juanicabanas
Copy link
Contributor

@juanicabanas juanicabanas commented May 3, 2024

What is this feature?

It wraps drawer component for rendering a scene component

Why do we need this feature?

It renders the drawer in Scene Dashboard. Specifically, for Sharing Modal redesign

Which issue(s) does this PR fix?:

Fixes #

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.

@juanicabanas juanicabanas added no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels May 3, 2024
@juanicabanas juanicabanas added this to the 11.0.x milestone May 3, 2024
@juanicabanas juanicabanas requested review from torkelo and a team as code owners May 3, 2024 19:10
@juanicabanas juanicabanas requested review from axelavargas and mdvictor and removed request for a team May 3, 2024 19:10
@grafana-delivery-bot grafana-delivery-bot bot modified the milestones: 11.0.x, 11.1.x May 3, 2024
@AgnesToulet AgnesToulet self-assigned this May 23, 2024
Copy link
Member

@torkelo torkelo left a comment

Choose a reason for hiding this comment

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

@juanicabanas not sure I understand the context / need for this?

Is the sharing drawer a scene? can't it render it's drawer itself? This SceneDrawer is only for embedding dashboard like views inside a drawer

@juanicabanas
Copy link
Contributor Author

@juanicabanas not sure I understand the context / need for this?

Is the sharing drawer a scene? can't it render it's drawer itself? This SceneDrawer is only for embedding dashboard like views inside a drawer

DashboardScene class has showModal method, which receives a SceneObject. We need the Drawer to be a SceneObject.
IINW, the Drawer children don't have to be a SceneObject necessary.
I tried to modify the component already created here that renders this children, so it can be re-used for scenes

@torkelo
Copy link
Member

torkelo commented May 27, 2024

@juanicabanas I am still unsure what your trying to do though, all the other uses of showModal passes a scene object that either renders a Drawer or Modal, so why the need to refactor the one uses by DataTrails?

@juanicabanas
Copy link
Contributor Author

@torkelo got it, I just checked and it's the scene object that renders a Modal or Drawer inside. So there's no need for this PR. Gonna close it

@grafana-delivery-bot grafana-delivery-bot bot removed this from the 11.1.x milestone May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes
Projects
Status: 🚀 Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants