Skip to content

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Jul 17, 2025

Description

Use newPlot if config has changed instead of _doPlot inside Plotly.react.

Supersedes #6395.

Changes

  • Adds check to determine if newPlot should be used
  • Updates tests per above change

Testing

  • Be on master
  • Launch Plotly DevTools
  • Open mock line_scatter (other mocks could work, but I know this one does)
  • Run the following snippet to update the mode bar buttons:
    let toggle = true
    
    Plotly.react(
      gd,
      gd.data,
      gd.layout,
      {
        displaylogo: false,
        modeBarButtonsToAdd: [
          {
            name: 'custombutton',
            title: toggle ? 'Take PICTURE' : 'Record MOVIE',
            icon: toggle ? Plotly.Icons.camera : Plotly.Icons.movie,
            click: () => {}
          },
        ],
        modeBarButtonsToRemove: [
          "autoscale",
          "pan2d",
          "lasso2d",
          "resetScale2d",
          "select2d",
          "toImage",
          "zoom",
          "zoomIn2d",
          "zoomOut2d"
        ]
      }
    )
  • Run this other snippet to update the label of the custom button:
    toggle = false
    
    Plotly.react(
      gd,
      gd.data,
      gd.layout,
      {
        displaylogo: false,
        modeBarButtonsToAdd: [
          {
            name: 'custombutton',
            title: toggle ? 'Take PICTURE' : 'Record MOVIE',
            icon: toggle ? Plotly.Icons.camera : Plotly.Icons.movie,
            click: () => {}
          },
        ],
        modeBarButtonsToRemove: [
          "autoscale",
          "pan2d",
          "lasso2d",
          "resetScale2d",
          "select2d",
          "toImage",
          "zoom",
          "zoomIn2d",
          "zoomOut2d"
        ]
      }
    )
  • Note that the button title and icon have not updated
  • Switch to this branch
  • Reload the mock
  • Run the two snippets again
  • Note that the button title and icon have changed

Notes

@archmoj archmoj requested a review from alexcjohnson July 17, 2025 21:04
@archmoj archmoj self-assigned this Jul 17, 2025
@archmoj archmoj added bug something broken cs customer success P1 needed for current cycle labels Jul 17, 2025
@archmoj archmoj requested a review from camdecoster July 23, 2025 01:08
Copy link
Contributor

@camdecoster camdecoster left a comment

Choose a reason for hiding this comment

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

This seems okay, but before I approve could you please provide a description and some steps to reproduce the issue? Then I can test before/after changes and see the fix in action.

@gvwilson gvwilson assigned camdecoster and unassigned archmoj Aug 8, 2025
@emilykl
Copy link
Contributor

emilykl commented Sep 3, 2025

@camdecoster This is intended to address #6394, correct?

Never mind, just saw your comment in the PR description. In that case, would it make sense to open another issue which can be closed by this PR?

Copy link
Contributor

@emilykl emilykl left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

Call react again to ensure transitions are triggered
Remove obsolete references to configChanged
@camdecoster
Copy link
Contributor

@camdecoster This is intended to address #6394, correct?

Never mind, just saw your comment in the PR description. In that case, would it make sense to open another issue which can be closed by this PR?

Yes. I have a comment drafted about that, but never submitted it. 🤦

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken cs customer success P1 needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants