This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Dont clone figure.layout - #905

Merged
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state
Jan 15, 2021
Merged

Dont clone figure.layout#905
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state

Conversation

@almarklein

@almarkleinalmarklein commented Dec 14, 2020

Copy link
Copy Markdown
Contributor

Fixes#879 (hopefully)

This small changes fixes the issue (for e.g. the example below). That said, I cannot oversee whether this potentially breaks other code. In theory, the tests will tell ;)

Example to test this:

importdashimportdash_html_componentsashtmlimportdash_core_componentsasdccfromdash.dependenciesimportInput, Output, Stateapp=dash.Dash(__name__, update_title=None)
fig= {"data": [], "layout": {"dragmode": "drawrect"}}
graph=dcc.Graph(id="graph", figure=fig)
app.layout=html.Div(
[
graph,
html.Br(),
html.Button(id='button', children="Clone figure"),
html.Div(id='output', children=""),
]
)
app.clientside_callback(
"""function clone_figure(_, figure) { let new_figure = {...figure}; let shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """,
[Output("graph", "figure"), Output("output", "children")],
[Input("button", "n_clicks")],
[State("graph", "figure")],
)
if__name__=="__main__":
app.run_server(debug=True)

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Mmm, there seems to be a problem with CI in general.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

hmm, not sure what's going on with CI, I'll have to investigate.

We're going to need to do more than just dropping the getLayout step though - check out what happens in there with responsive and how it modifies autosize, height, and width.

In principle we may need to save the original values of these attributes in state, so cases where the user provides a figure and then later changes the responsive prop without changing the figure itself we can bring those original values back as needed. In practice though I doubt users change responsive much.

Comment threadsrc/fragments/Graph.react.js Outdated
@alexcjohnsonalexcjohnson mentioned this pull request Dec 15, 2020
1 task
@alexcjohnson

Copy link
Copy Markdown
Collaborator

Mmm, there seems to be a problem with CI in general.

The dash-html-components build process broke. I'm investigating and will update you when it's resolved.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Fixed the build process via plotly/dash-html-components#170 - now it's just routine linter errors 😏

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated, and tests added. Only a linting issue with prettier. Unfortunately, it does not show what is wrong, and I cannot reproduce the linting locally (and I do not see any obvious errors).

@almarklein

Copy link
Copy Markdown
ContributorAuthor

I also updated the code sample in the top post to do about the same thing as the test does.

for this PR and the import fix
@alexcjohnson

Copy link
Copy Markdown
Collaborator

eb82ae1 resulted from npm run format (fixed the prettier issues), but also for some reason locally I needed to update the __init__.py before the pre-commit hooks would pass. 🤷

Comment threadsrc/fragments/Graph.react.js Outdated
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated again.

I also ran a little test with a layout that has width and height set to 300. The figure is originally this small size. Turning responsive to true will make the figure span the available space (and width and height are undefined). Turning responsive off again resets the figure to its original small size.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

The failing test looks like a glitch in another test.

@emmanuelle

Copy link
Copy Markdown
Contributor

I restarted the CI and all looks good now.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson also added a test to check that original values are correctly restored.

Comment threadtests/integration/graph/test_graph_varia.py Outdated
Comment threadtests/integration/graph/test_graph_varia.py Outdated
@emmanuelle

Copy link
Copy Markdown
Contributor

Hi, any chance to get this PR merged before the next Dash release? Pretty please :-) ?

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Yes sorry, it's been waiting on me to play with it a bit, but we'll get it in for the next release.

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK! @almarklein I was concerned about how this would work in various other cases including both mutations and brand new figures and alternating responsive and figure edits, so I expanded on your second test a bit. All works great! 💃

@alexcjohnson
alexcjohnson merged commit 414be44 into plotly:devJan 15, 2021
@almarklein
almarklein deleted the figure-state branch January 15, 2021 14:53
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Thanks @alexcjohnson for wrapping this one up :)

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

figure as state out of sync

3 participants

@almarklein@alexcjohnson@emmanuelle
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Dont clone figure.layout - #905

Merged
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state
Jan 15, 2021
Merged

Dont clone figure.layout#905
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state

Conversation

@almarklein

@almarkleinalmarklein commented Dec 14, 2020

Copy link
Copy Markdown
Contributor

Fixes#879 (hopefully)

This small changes fixes the issue (for e.g. the example below). That said, I cannot oversee whether this potentially breaks other code. In theory, the tests will tell ;)

Example to test this:

importdashimportdash_html_componentsashtmlimportdash_core_componentsasdccfromdash.dependenciesimportInput, Output, Stateapp=dash.Dash(__name__, update_title=None)
fig= {"data": [], "layout": {"dragmode": "drawrect"}}
graph=dcc.Graph(id="graph", figure=fig)
app.layout=html.Div(
[
graph,
html.Br(),
html.Button(id='button', children="Clone figure"),
html.Div(id='output', children=""),
]
)
app.clientside_callback(
"""function clone_figure(_, figure) { let new_figure = {...figure}; let shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """,
[Output("graph", "figure"), Output("output", "children")],
[Input("button", "n_clicks")],
[State("graph", "figure")],
)
if__name__=="__main__":
app.run_server(debug=True)

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Mmm, there seems to be a problem with CI in general.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

hmm, not sure what's going on with CI, I'll have to investigate.

We're going to need to do more than just dropping the getLayout step though - check out what happens in there with responsive and how it modifies autosize, height, and width.

In principle we may need to save the original values of these attributes in state, so cases where the user provides a figure and then later changes the responsive prop without changing the figure itself we can bring those original values back as needed. In practice though I doubt users change responsive much.

Comment threadsrc/fragments/Graph.react.js Outdated
@alexcjohnsonalexcjohnson mentioned this pull request Dec 15, 2020
1 task
@alexcjohnson

Copy link
Copy Markdown
Collaborator

Mmm, there seems to be a problem with CI in general.

The dash-html-components build process broke. I'm investigating and will update you when it's resolved.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Fixed the build process via plotly/dash-html-components#170 - now it's just routine linter errors 😏

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated, and tests added. Only a linting issue with prettier. Unfortunately, it does not show what is wrong, and I cannot reproduce the linting locally (and I do not see any obvious errors).

@almarklein

Copy link
Copy Markdown
ContributorAuthor

I also updated the code sample in the top post to do about the same thing as the test does.

for this PR and the import fix
@alexcjohnson

Copy link
Copy Markdown
Collaborator

eb82ae1 resulted from npm run format (fixed the prettier issues), but also for some reason locally I needed to update the __init__.py before the pre-commit hooks would pass. 🤷

Comment threadsrc/fragments/Graph.react.js Outdated
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated again.

I also ran a little test with a layout that has width and height set to 300. The figure is originally this small size. Turning responsive to true will make the figure span the available space (and width and height are undefined). Turning responsive off again resets the figure to its original small size.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

The failing test looks like a glitch in another test.

@emmanuelle

Copy link
Copy Markdown
Contributor

I restarted the CI and all looks good now.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson also added a test to check that original values are correctly restored.

Comment threadtests/integration/graph/test_graph_varia.py Outdated
Comment threadtests/integration/graph/test_graph_varia.py Outdated
@emmanuelle

Copy link
Copy Markdown
Contributor

Hi, any chance to get this PR merged before the next Dash release? Pretty please :-) ?

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Yes sorry, it's been waiting on me to play with it a bit, but we'll get it in for the next release.

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK! @almarklein I was concerned about how this would work in various other cases including both mutations and brand new figures and alternating responsive and figure edits, so I expanded on your second test a bit. All works great! 💃

@alexcjohnson
alexcjohnson merged commit 414be44 into plotly:devJan 15, 2021
@almarklein
almarklein deleted the figure-state branch January 15, 2021 14:53
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Thanks @alexcjohnson for wrapping this one up :)

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

figure as state out of sync

3 participants

@almarklein@alexcjohnson@emmanuelle
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Dont clone figure.layout - #905

Merged
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state
Jan 15, 2021
Merged

Dont clone figure.layout#905
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state

Conversation

@almarklein

@almarkleinalmarklein commented Dec 14, 2020

Copy link
Copy Markdown
Contributor

Fixes#879 (hopefully)

This small changes fixes the issue (for e.g. the example below). That said, I cannot oversee whether this potentially breaks other code. In theory, the tests will tell ;)

Example to test this:

importdashimportdash_html_componentsashtmlimportdash_core_componentsasdccfromdash.dependenciesimportInput, Output, Stateapp=dash.Dash(__name__, update_title=None)
fig= {"data": [], "layout": {"dragmode": "drawrect"}}
graph=dcc.Graph(id="graph", figure=fig)
app.layout=html.Div(
[
graph,
html.Br(),
html.Button(id='button', children="Clone figure"),
html.Div(id='output', children=""),
]
)
app.clientside_callback(
"""function clone_figure(_, figure) { let new_figure = {...figure}; let shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """,
[Output("graph", "figure"), Output("output", "children")],
[Input("button", "n_clicks")],
[State("graph", "figure")],
)
if__name__=="__main__":
app.run_server(debug=True)

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Mmm, there seems to be a problem with CI in general.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

hmm, not sure what's going on with CI, I'll have to investigate.

We're going to need to do more than just dropping the getLayout step though - check out what happens in there with responsive and how it modifies autosize, height, and width.

In principle we may need to save the original values of these attributes in state, so cases where the user provides a figure and then later changes the responsive prop without changing the figure itself we can bring those original values back as needed. In practice though I doubt users change responsive much.

Comment threadsrc/fragments/Graph.react.js Outdated
@alexcjohnsonalexcjohnson mentioned this pull request Dec 15, 2020
1 task
@alexcjohnson

Copy link
Copy Markdown
Collaborator

Mmm, there seems to be a problem with CI in general.

The dash-html-components build process broke. I'm investigating and will update you when it's resolved.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Fixed the build process via plotly/dash-html-components#170 - now it's just routine linter errors 😏

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated, and tests added. Only a linting issue with prettier. Unfortunately, it does not show what is wrong, and I cannot reproduce the linting locally (and I do not see any obvious errors).

@almarklein

Copy link
Copy Markdown
ContributorAuthor

I also updated the code sample in the top post to do about the same thing as the test does.

for this PR and the import fix
@alexcjohnson

Copy link
Copy Markdown
Collaborator

eb82ae1 resulted from npm run format (fixed the prettier issues), but also for some reason locally I needed to update the __init__.py before the pre-commit hooks would pass. 🤷

Comment threadsrc/fragments/Graph.react.js Outdated
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated again.

I also ran a little test with a layout that has width and height set to 300. The figure is originally this small size. Turning responsive to true will make the figure span the available space (and width and height are undefined). Turning responsive off again resets the figure to its original small size.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

The failing test looks like a glitch in another test.

@emmanuelle

Copy link
Copy Markdown
Contributor

I restarted the CI and all looks good now.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson also added a test to check that original values are correctly restored.

Comment threadtests/integration/graph/test_graph_varia.py Outdated
Comment threadtests/integration/graph/test_graph_varia.py Outdated
@emmanuelle

Copy link
Copy Markdown
Contributor

Hi, any chance to get this PR merged before the next Dash release? Pretty please :-) ?

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Yes sorry, it's been waiting on me to play with it a bit, but we'll get it in for the next release.

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK! @almarklein I was concerned about how this would work in various other cases including both mutations and brand new figures and alternating responsive and figure edits, so I expanded on your second test a bit. All works great! 💃

@alexcjohnson
alexcjohnson merged commit 414be44 into plotly:devJan 15, 2021
@almarklein
almarklein deleted the figure-state branch January 15, 2021 14:53
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Thanks @alexcjohnson for wrapping this one up :)

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

figure as state out of sync

3 participants

@almarklein@alexcjohnson@emmanuelle
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Dont clone figure.layout - #905

Merged
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state
Jan 15, 2021
Merged

Dont clone figure.layout#905
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state

Conversation

@almarklein

@almarkleinalmarklein commented Dec 14, 2020

Copy link
Copy Markdown
Contributor

Fixes#879 (hopefully)

This small changes fixes the issue (for e.g. the example below). That said, I cannot oversee whether this potentially breaks other code. In theory, the tests will tell ;)

Example to test this:

importdashimportdash_html_componentsashtmlimportdash_core_componentsasdccfromdash.dependenciesimportInput, Output, Stateapp=dash.Dash(__name__, update_title=None)
fig= {"data": [], "layout": {"dragmode": "drawrect"}}
graph=dcc.Graph(id="graph", figure=fig)
app.layout=html.Div(
[
graph,
html.Br(),
html.Button(id='button', children="Clone figure"),
html.Div(id='output', children=""),
]
)
app.clientside_callback(
"""function clone_figure(_, figure) { let new_figure = {...figure}; let shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """,
[Output("graph", "figure"), Output("output", "children")],
[Input("button", "n_clicks")],
[State("graph", "figure")],
)
if__name__=="__main__":
app.run_server(debug=True)

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Mmm, there seems to be a problem with CI in general.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

hmm, not sure what's going on with CI, I'll have to investigate.

We're going to need to do more than just dropping the getLayout step though - check out what happens in there with responsive and how it modifies autosize, height, and width.

In principle we may need to save the original values of these attributes in state, so cases where the user provides a figure and then later changes the responsive prop without changing the figure itself we can bring those original values back as needed. In practice though I doubt users change responsive much.

Comment threadsrc/fragments/Graph.react.js Outdated
@alexcjohnsonalexcjohnson mentioned this pull request Dec 15, 2020
1 task
@alexcjohnson

Copy link
Copy Markdown
Collaborator

Mmm, there seems to be a problem with CI in general.

The dash-html-components build process broke. I'm investigating and will update you when it's resolved.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Fixed the build process via plotly/dash-html-components#170 - now it's just routine linter errors 😏

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated, and tests added. Only a linting issue with prettier. Unfortunately, it does not show what is wrong, and I cannot reproduce the linting locally (and I do not see any obvious errors).

@almarklein

Copy link
Copy Markdown
ContributorAuthor

I also updated the code sample in the top post to do about the same thing as the test does.

for this PR and the import fix
@alexcjohnson

Copy link
Copy Markdown
Collaborator

eb82ae1 resulted from npm run format (fixed the prettier issues), but also for some reason locally I needed to update the __init__.py before the pre-commit hooks would pass. 🤷

Comment threadsrc/fragments/Graph.react.js Outdated
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated again.

I also ran a little test with a layout that has width and height set to 300. The figure is originally this small size. Turning responsive to true will make the figure span the available space (and width and height are undefined). Turning responsive off again resets the figure to its original small size.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

The failing test looks like a glitch in another test.

@emmanuelle

Copy link
Copy Markdown
Contributor

I restarted the CI and all looks good now.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson also added a test to check that original values are correctly restored.

Comment threadtests/integration/graph/test_graph_varia.py Outdated
Comment threadtests/integration/graph/test_graph_varia.py Outdated
@emmanuelle

Copy link
Copy Markdown
Contributor

Hi, any chance to get this PR merged before the next Dash release? Pretty please :-) ?

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Yes sorry, it's been waiting on me to play with it a bit, but we'll get it in for the next release.

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK! @almarklein I was concerned about how this would work in various other cases including both mutations and brand new figures and alternating responsive and figure edits, so I expanded on your second test a bit. All works great! 💃

@alexcjohnson
alexcjohnson merged commit 414be44 into plotly:devJan 15, 2021
@almarklein
almarklein deleted the figure-state branch January 15, 2021 14:53
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Thanks @alexcjohnson for wrapping this one up :)

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

figure as state out of sync

3 participants

@almarklein@alexcjohnson@emmanuelle
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Dont clone figure.layout - #905

Merged
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state
Jan 15, 2021
Merged

Dont clone figure.layout#905
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state

Conversation

@almarklein

@almarkleinalmarklein commented Dec 14, 2020

Copy link
Copy Markdown
Contributor

Fixes#879 (hopefully)

This small changes fixes the issue (for e.g. the example below). That said, I cannot oversee whether this potentially breaks other code. In theory, the tests will tell ;)

Example to test this:

importdashimportdash_html_componentsashtmlimportdash_core_componentsasdccfromdash.dependenciesimportInput, Output, Stateapp=dash.Dash(__name__, update_title=None)
fig= {"data": [], "layout": {"dragmode": "drawrect"}}
graph=dcc.Graph(id="graph", figure=fig)
app.layout=html.Div(
[
graph,
html.Br(),
html.Button(id='button', children="Clone figure"),
html.Div(id='output', children=""),
]
)
app.clientside_callback(
"""function clone_figure(_, figure) { let new_figure = {...figure}; let shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """,
[Output("graph", "figure"), Output("output", "children")],
[Input("button", "n_clicks")],
[State("graph", "figure")],
)
if__name__=="__main__":
app.run_server(debug=True)

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Mmm, there seems to be a problem with CI in general.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

hmm, not sure what's going on with CI, I'll have to investigate.

We're going to need to do more than just dropping the getLayout step though - check out what happens in there with responsive and how it modifies autosize, height, and width.

In principle we may need to save the original values of these attributes in state, so cases where the user provides a figure and then later changes the responsive prop without changing the figure itself we can bring those original values back as needed. In practice though I doubt users change responsive much.

Comment threadsrc/fragments/Graph.react.js Outdated
@alexcjohnsonalexcjohnson mentioned this pull request Dec 15, 2020
1 task
@alexcjohnson

Copy link
Copy Markdown
Collaborator

Mmm, there seems to be a problem with CI in general.

The dash-html-components build process broke. I'm investigating and will update you when it's resolved.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Fixed the build process via plotly/dash-html-components#170 - now it's just routine linter errors 😏

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated, and tests added. Only a linting issue with prettier. Unfortunately, it does not show what is wrong, and I cannot reproduce the linting locally (and I do not see any obvious errors).

@almarklein

Copy link
Copy Markdown
ContributorAuthor

I also updated the code sample in the top post to do about the same thing as the test does.

for this PR and the import fix
@alexcjohnson

Copy link
Copy Markdown
Collaborator

eb82ae1 resulted from npm run format (fixed the prettier issues), but also for some reason locally I needed to update the __init__.py before the pre-commit hooks would pass. 🤷

Comment threadsrc/fragments/Graph.react.js Outdated
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated again.

I also ran a little test with a layout that has width and height set to 300. The figure is originally this small size. Turning responsive to true will make the figure span the available space (and width and height are undefined). Turning responsive off again resets the figure to its original small size.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

The failing test looks like a glitch in another test.

@emmanuelle

Copy link
Copy Markdown
Contributor

I restarted the CI and all looks good now.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson also added a test to check that original values are correctly restored.

Comment threadtests/integration/graph/test_graph_varia.py Outdated
Comment threadtests/integration/graph/test_graph_varia.py Outdated
@emmanuelle

Copy link
Copy Markdown
Contributor

Hi, any chance to get this PR merged before the next Dash release? Pretty please :-) ?

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Yes sorry, it's been waiting on me to play with it a bit, but we'll get it in for the next release.

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK! @almarklein I was concerned about how this would work in various other cases including both mutations and brand new figures and alternating responsive and figure edits, so I expanded on your second test a bit. All works great! 💃

@alexcjohnson
alexcjohnson merged commit 414be44 into plotly:devJan 15, 2021
@almarklein
almarklein deleted the figure-state branch January 15, 2021 14:53
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Thanks @alexcjohnson for wrapping this one up :)

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

figure as state out of sync

3 participants

@almarklein@alexcjohnson@emmanuelle
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Dont clone figure.layout - #905

Merged
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state
Jan 15, 2021
Merged

Dont clone figure.layout#905
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state

Conversation

@almarklein

@almarkleinalmarklein commented Dec 14, 2020

Copy link
Copy Markdown
Contributor

Fixes#879 (hopefully)

This small changes fixes the issue (for e.g. the example below). That said, I cannot oversee whether this potentially breaks other code. In theory, the tests will tell ;)

Example to test this:

importdashimportdash_html_componentsashtmlimportdash_core_componentsasdccfromdash.dependenciesimportInput, Output, Stateapp=dash.Dash(__name__, update_title=None)
fig= {"data": [], "layout": {"dragmode": "drawrect"}}
graph=dcc.Graph(id="graph", figure=fig)
app.layout=html.Div(
[
graph,
html.Br(),
html.Button(id='button', children="Clone figure"),
html.Div(id='output', children=""),
]
)
app.clientside_callback(
"""function clone_figure(_, figure) { let new_figure = {...figure}; let shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """,
[Output("graph", "figure"), Output("output", "children")],
[Input("button", "n_clicks")],
[State("graph", "figure")],
)
if__name__=="__main__":
app.run_server(debug=True)

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Mmm, there seems to be a problem with CI in general.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

hmm, not sure what's going on with CI, I'll have to investigate.

We're going to need to do more than just dropping the getLayout step though - check out what happens in there with responsive and how it modifies autosize, height, and width.

In principle we may need to save the original values of these attributes in state, so cases where the user provides a figure and then later changes the responsive prop without changing the figure itself we can bring those original values back as needed. In practice though I doubt users change responsive much.

Comment threadsrc/fragments/Graph.react.js Outdated
@alexcjohnsonalexcjohnson mentioned this pull request Dec 15, 2020
1 task
@alexcjohnson

Copy link
Copy Markdown
Collaborator

Mmm, there seems to be a problem with CI in general.

The dash-html-components build process broke. I'm investigating and will update you when it's resolved.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Fixed the build process via plotly/dash-html-components#170 - now it's just routine linter errors 😏

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated, and tests added. Only a linting issue with prettier. Unfortunately, it does not show what is wrong, and I cannot reproduce the linting locally (and I do not see any obvious errors).

@almarklein

Copy link
Copy Markdown
ContributorAuthor

I also updated the code sample in the top post to do about the same thing as the test does.

for this PR and the import fix
@alexcjohnson

Copy link
Copy Markdown
Collaborator

eb82ae1 resulted from npm run format (fixed the prettier issues), but also for some reason locally I needed to update the __init__.py before the pre-commit hooks would pass. 🤷

Comment threadsrc/fragments/Graph.react.js Outdated
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated again.

I also ran a little test with a layout that has width and height set to 300. The figure is originally this small size. Turning responsive to true will make the figure span the available space (and width and height are undefined). Turning responsive off again resets the figure to its original small size.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

The failing test looks like a glitch in another test.

@emmanuelle

Copy link
Copy Markdown
Contributor

I restarted the CI and all looks good now.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson also added a test to check that original values are correctly restored.

Comment threadtests/integration/graph/test_graph_varia.py Outdated
Comment threadtests/integration/graph/test_graph_varia.py Outdated
@emmanuelle

Copy link
Copy Markdown
Contributor

Hi, any chance to get this PR merged before the next Dash release? Pretty please :-) ?

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Yes sorry, it's been waiting on me to play with it a bit, but we'll get it in for the next release.

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK! @almarklein I was concerned about how this would work in various other cases including both mutations and brand new figures and alternating responsive and figure edits, so I expanded on your second test a bit. All works great! 💃

@alexcjohnson
alexcjohnson merged commit 414be44 into plotly:devJan 15, 2021
@almarklein
almarklein deleted the figure-state branch January 15, 2021 14:53
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Thanks @alexcjohnson for wrapping this one up :)

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

figure as state out of sync

3 participants

@almarklein@alexcjohnson@emmanuelle
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Dont clone figure.layout - #905

Merged
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state
Jan 15, 2021
Merged

Dont clone figure.layout#905
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state

Conversation

@almarklein

@almarkleinalmarklein commented Dec 14, 2020

Copy link
Copy Markdown
Contributor

Fixes#879 (hopefully)

This small changes fixes the issue (for e.g. the example below). That said, I cannot oversee whether this potentially breaks other code. In theory, the tests will tell ;)

Example to test this:

importdashimportdash_html_componentsashtmlimportdash_core_componentsasdccfromdash.dependenciesimportInput, Output, Stateapp=dash.Dash(__name__, update_title=None)
fig= {"data": [], "layout": {"dragmode": "drawrect"}}
graph=dcc.Graph(id="graph", figure=fig)
app.layout=html.Div(
[
graph,
html.Br(),
html.Button(id='button', children="Clone figure"),
html.Div(id='output', children=""),
]
)
app.clientside_callback(
"""function clone_figure(_, figure) { let new_figure = {...figure}; let shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """,
[Output("graph", "figure"), Output("output", "children")],
[Input("button", "n_clicks")],
[State("graph", "figure")],
)
if__name__=="__main__":
app.run_server(debug=True)

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Mmm, there seems to be a problem with CI in general.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

hmm, not sure what's going on with CI, I'll have to investigate.

We're going to need to do more than just dropping the getLayout step though - check out what happens in there with responsive and how it modifies autosize, height, and width.

In principle we may need to save the original values of these attributes in state, so cases where the user provides a figure and then later changes the responsive prop without changing the figure itself we can bring those original values back as needed. In practice though I doubt users change responsive much.

Comment threadsrc/fragments/Graph.react.js Outdated
@alexcjohnsonalexcjohnson mentioned this pull request Dec 15, 2020
1 task
@alexcjohnson

Copy link
Copy Markdown
Collaborator

Mmm, there seems to be a problem with CI in general.

The dash-html-components build process broke. I'm investigating and will update you when it's resolved.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Fixed the build process via plotly/dash-html-components#170 - now it's just routine linter errors 😏

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated, and tests added. Only a linting issue with prettier. Unfortunately, it does not show what is wrong, and I cannot reproduce the linting locally (and I do not see any obvious errors).

@almarklein

Copy link
Copy Markdown
ContributorAuthor

I also updated the code sample in the top post to do about the same thing as the test does.

for this PR and the import fix
@alexcjohnson

Copy link
Copy Markdown
Collaborator

eb82ae1 resulted from npm run format (fixed the prettier issues), but also for some reason locally I needed to update the __init__.py before the pre-commit hooks would pass. 🤷

Comment threadsrc/fragments/Graph.react.js Outdated
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated again.

I also ran a little test with a layout that has width and height set to 300. The figure is originally this small size. Turning responsive to true will make the figure span the available space (and width and height are undefined). Turning responsive off again resets the figure to its original small size.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

The failing test looks like a glitch in another test.

@emmanuelle

Copy link
Copy Markdown
Contributor

I restarted the CI and all looks good now.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson also added a test to check that original values are correctly restored.

Comment threadtests/integration/graph/test_graph_varia.py Outdated
Comment threadtests/integration/graph/test_graph_varia.py Outdated
@emmanuelle

Copy link
Copy Markdown
Contributor

Hi, any chance to get this PR merged before the next Dash release? Pretty please :-) ?

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Yes sorry, it's been waiting on me to play with it a bit, but we'll get it in for the next release.

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK! @almarklein I was concerned about how this would work in various other cases including both mutations and brand new figures and alternating responsive and figure edits, so I expanded on your second test a bit. All works great! 💃

@alexcjohnson
alexcjohnson merged commit 414be44 into plotly:devJan 15, 2021
@almarklein
almarklein deleted the figure-state branch January 15, 2021 14:53
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Thanks @alexcjohnson for wrapping this one up :)

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

figure as state out of sync

3 participants

@almarklein@alexcjohnson@emmanuelle
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Dont clone figure.layout - #905

Merged
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state
Jan 15, 2021
Merged

Dont clone figure.layout#905
alexcjohnson merged 16 commits into
plotly:devfrom
almarklein:figure-state

Conversation

@almarklein

@almarkleinalmarklein commented Dec 14, 2020

Copy link
Copy Markdown
Contributor

Fixes#879 (hopefully)

This small changes fixes the issue (for e.g. the example below). That said, I cannot oversee whether this potentially breaks other code. In theory, the tests will tell ;)

Example to test this:

importdashimportdash_html_componentsashtmlimportdash_core_componentsasdccfromdash.dependenciesimportInput, Output, Stateapp=dash.Dash(__name__, update_title=None)
fig= {"data": [], "layout": {"dragmode": "drawrect"}}
graph=dcc.Graph(id="graph", figure=fig)
app.layout=html.Div(
[
graph,
html.Br(),
html.Button(id='button', children="Clone figure"),
html.Div(id='output', children=""),
]
)
app.clientside_callback(
"""function clone_figure(_, figure) { let new_figure = {...figure}; let shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """,
[Output("graph", "figure"), Output("output", "children")],
[Input("button", "n_clicks")],
[State("graph", "figure")],
)
if__name__=="__main__":
app.run_server(debug=True)

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Mmm, there seems to be a problem with CI in general.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

hmm, not sure what's going on with CI, I'll have to investigate.

We're going to need to do more than just dropping the getLayout step though - check out what happens in there with responsive and how it modifies autosize, height, and width.

In principle we may need to save the original values of these attributes in state, so cases where the user provides a figure and then later changes the responsive prop without changing the figure itself we can bring those original values back as needed. In practice though I doubt users change responsive much.

Comment threadsrc/fragments/Graph.react.js Outdated
@alexcjohnsonalexcjohnson mentioned this pull request Dec 15, 2020
1 task
@alexcjohnson

Copy link
Copy Markdown
Collaborator

Mmm, there seems to be a problem with CI in general.

The dash-html-components build process broke. I'm investigating and will update you when it's resolved.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Fixed the build process via plotly/dash-html-components#170 - now it's just routine linter errors 😏

@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated, and tests added. Only a linting issue with prettier. Unfortunately, it does not show what is wrong, and I cannot reproduce the linting locally (and I do not see any obvious errors).

@almarklein

Copy link
Copy Markdown
ContributorAuthor

I also updated the code sample in the top post to do about the same thing as the test does.

for this PR and the import fix
@alexcjohnson

Copy link
Copy Markdown
Collaborator

eb82ae1 resulted from npm run format (fixed the prettier issues), but also for some reason locally I needed to update the __init__.py before the pre-commit hooks would pass. 🤷

Comment threadsrc/fragments/Graph.react.js Outdated
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Updated again.

I also ran a little test with a layout that has width and height set to 300. The figure is originally this small size. Turning responsive to true will make the figure span the available space (and width and height are undefined). Turning responsive off again resets the figure to its original small size.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

The failing test looks like a glitch in another test.

@emmanuelle

Copy link
Copy Markdown
Contributor

I restarted the CI and all looks good now.

@almarklein

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson also added a test to check that original values are correctly restored.

Comment threadtests/integration/graph/test_graph_varia.py Outdated
Comment threadtests/integration/graph/test_graph_varia.py Outdated
@emmanuelle

Copy link
Copy Markdown
Contributor

Hi, any chance to get this PR merged before the next Dash release? Pretty please :-) ?

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Yes sorry, it's been waiting on me to play with it a bit, but we'll get it in for the next release.

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK! @almarklein I was concerned about how this would work in various other cases including both mutations and brand new figures and alternating responsive and figure edits, so I expanded on your second test a bit. All works great! 💃

@alexcjohnson
alexcjohnson merged commit 414be44 into plotly:devJan 15, 2021
@almarklein
almarklein deleted the figure-state branch January 15, 2021 14:53
@almarklein

Copy link
Copy Markdown
ContributorAuthor

Thanks @alexcjohnson for wrapping this one up :)

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

figure as state out of sync

3 participants

@almarklein@alexcjohnson@emmanuelle