feat: Add chart event for title change#2352
Conversation
| } | ||
| case ChartModel.EVENT_TITLE_CHANGE: { | ||
| const titleText = `${detail}`; | ||
| const oldTitle = this.state.layout.title; |
There was a problem hiding this comment.
| const oldTitle = this.state.layout.title; | |
| const { title: oldTitle } = this.state.layout; |
| this.setState({ shownBlocker: null }); | ||
| break; | ||
| } | ||
| case ChartModel.EVENT_TITLE_CHANGE: { |
There was a problem hiding this comment.
Instead of an EVENT_TITLE_CHANGE, go with EVENT_LAYOUT_UPDATED and emit the new layout.
ChartModel already has the layout (.getLayout()) and that should contain the updated title.
Also see in ChartModel.initData how we use the chart layout on top of the state layout... so this would be something like:
case ChartModel.EVENT_LAYOUT_UPDATED: {
const newLayout = detail as Partial<Layout>;
this.setState(({ layout, revision }) => ({ layout: { ...layout, ...newLayout }, revision: revision + 1 }))
}
|
@jnumainville also if you could separate the decimal fix into a separate PR that would make cherry-picking changes if needed easier. |
|
format options added to #2353 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2352 +/- ##
==========================================
- Coverage 46.09% 46.09% -0.01%
==========================================
Files 712 712
Lines 39674 39681 +7
Branches 10055 9870 -185
==========================================
+ Hits 18288 18289 +1
- Misses 21375 21381 +6
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
mofojed
left a comment
mofojed
left a comment
There was a problem hiding this comment.
Please merge the latest main into this branch
jnumainville
changed the title
2 participants
These are for deephaven/deephaven-plugins#1088
Currently the layout is only set once, but with indicator it may change, although specifically the title, so I added an event for that. Not sure if it's the best approach.