Hide undo redo #175
Hide undo redo #175
Conversation
| _dash_app_content_html = os.path.join( | ||
| os.path.dirname(__file__), | ||
| 'test_assets', 'initial_state_dash_app_content.html') | ||
| with open(_dash_app_content_html) as fp: | ||
| rendered_dom = BeautifulSoup(fp.read(), 'lxml') | ||
| rendered_dom = BeautifulSoup(fp.read().strip(), 'lxml') |
There was a problem hiding this comment.
strip because my editor doesn't want to save files without a trailing newline
| }) | ||
| ); | ||
| }; | ||
| return undo_revert(UNDO); |
There was a problem hiding this comment.
DRYing this section didn't change the functionality at all... but I discovered I could while researching another question: Can we remove the history from the store entirely when these buttons are hidden? The answer to that is no: in principle we could simplify it to saving just the last past entry and nothing else - which may be a worthwhile improvement for very large and long-running apps, it's possible the memory usage will eventually cause problems. But revert is still needed for error recovery.
There was a problem hiding this comment.
Could you open an issue to investigate/implement the improvement? Wonder how much of an issue this could be today with long-running apps updating graph/table components.
There was a problem hiding this comment.
Could you open an issue to investigate/implement the improvement? Wonder how much of an issue this could be today with long-running apps updating graph/table components.
| return ( | ||
| <React.Fragment> | ||
| <Toolbar /> | ||
| {show_undo_redo ? <Toolbar /> : null} |
There was a problem hiding this comment.
Maybe a note in the Changelog to the effect that that the CSS solution is now obsolete.
There was a problem hiding this comment.
changelog: e1941e1
| @@ -154,11 +154,12 @@ def test_initial_state(self): | |||
| self.startServer(app) | |||
| el = self.wait_for_element_by_css_selector('#react-entry-point') | |||
|
|
|||
| # Note: this .html file shows there's no undo/redo button by default | |||
There was a problem hiding this comment.
Due to visual tests instability I would prefer to isolate this behavior in its own test instead of grafting it to an existing one. Specifically, testing that undo & redo buttons are not found in the DOM + a percy snapshot.
There was a problem hiding this comment.
I'll include a test that these elements aren't in the DOM. The test I already added includes a percy snapshot with undo AND redo buttons - I guess for completeness I could extend it test that at either end of the history the appropriate button disappears, I'll do that - but we now have a million percy snapshots that used to have undo buttons and no longer do.
There was a problem hiding this comment.
extended element existence tests -> 56cfddc
Companion to plotly/dash#724