-
Notifications
You must be signed in to change notification settings - Fork 11.7k
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
Canvas: Adhere editing state to dashboard editable state #87883
Conversation
const dashboard = getDashboardSrv().getCurrent(); | ||
const allowEditing = this.props.options.inlineEditing && dashboard?.editable; | ||
|
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.
editable is not the same as isEditing,
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.
but you could add a new property isEditing on DashboardModelCompatibilityWrapper if you want
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.
In its current form this PR addresses the legacy dashboard case which I think is still worth addressing as dashboard scenes is still in public preview
Created this follow up GitHub issue to address figuring this out for new dashboards based on scenes. I think for this to work as expected I think the dashboard scene will need to emit a state update (like editing is turned off / on) that a panel can listen to and respond to accordingly
Thanks for the quick fix! (I reported it to @leventebalogh on Slack) |
Adhere canvas editing state to dashboard's editable state. Before if the dashboard was read-only, users could still modify the canvas but would then have no way to save those changes.
Note: this approach will need to be updated for dashboard scene migration, but given as that is currently in public preview it makes sense to fix classic dashboards for the time being
See this follow up GitHub issue
Before
Screen.Recording.2024-05-07.at.15.27.59.mov
After
after.mov
Fixes #87446