Add ScrollSync Button to Preview UI #693
Conversation
|
Sorry, should have waited until #659 was merged. |
|
Thanks @Jinksi! I want to improve the arch on this, mostly just creating a single toggle component and reusing that, and avoiding the universal selector in CSS. If you have time to handle that and that rebase we can get this in sooner, otherwise I or another contributor will get to it. |
|
@erquhart No problem, I'll get onto it |
|
Sorry, looks like I messed up the rebase |
|
Rebase fixed. |
There was a problem hiding this comment.
Looking good! Just a few changes and this is good to go. This improvement did get our control panel buttons to a place where they merit a separate component, that's worth doing in the future.
I'll also note for any other reviewers that this is only necessary because our scroll syncing is very simplistic. The real fix is to make it more robust, but that'll take some doing, and no one is working on it at the moment. Just know that we're not throwing in the towel on that.
| @@ -51,6 +52,7 @@ export default class ScrollSync extends Component { | |||
| }; | |||
|
|
|||
| handlePaneScroll = (node) => { | |||
| if (!this.props.enabled) return false | |||
There was a problem hiding this comment.
Add semi-colon at line end.
| @@ -6,6 +6,7 @@ export default class ScrollSync extends Component { | |||
|
|
|||
| static propTypes = { | |||
| children: PropTypes.element.isRequired, | |||
| enabled: PropTypes.bool.isRequired | |||
There was a problem hiding this comment.
Add trailing comma to last item in a collection.
| <div className="nc-entryEditor-controlPaneButtons"> | ||
| { previewVisible && ( | ||
| <ToggleButton | ||
| icon={scrollSyncEnabled ? 'sync_disabled' : 'sync'} |
There was a problem hiding this comment.
These need to be swapped, currently shows the disabled icon when scrolling is enabled.
* Add ScrollSync Button * Create <ToggleButton /> component && update CSS * Swap Icons && fix formatting
Closes #532
- Summary
syncandsync-disabledicon- Test plan
Manually tested UI (see screen capture). If I should be adding formal tests, please let me know.
- Description for the changelog
Added button to toggle preview scroll-sync.
- A picture of a cute animal (not mandatory but encouraged)