New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force HTML blocks to be in preview mode when in a shared block #7138

Merged
merged 1 commit into from Jun 8, 2018

Conversation

Projects
None yet
3 participants
@noisysocks
Member

noisysocks commented Jun 5, 2018

Fixes #4875. Supersedes #5298.

Introduces Disabled.Consumer so that components are able to tell if they are wrapped in a <Disabled> component.

We then use this functionality to force HTML blocks to render in preview mode when disabled. This means that HTML blocks always render in preview mode when they are shared which is a nice UX enhancement.

html-preview

How has this been tested?

I unfortunately had to temporarily skip all of the Disabled tests because Enzyme does not support React 16.3 APIs as of yet (airbnb/enzyme#1513).

I manually tested by:

  1. Create a HTML block
  2. Convert it to a shared block—it should now be in preview mode
  3. Edit the shared block—it should now be in edit mode
  4. Toggle the block between preview and edit mode
Force HTML blocks to be in preview mode when in a shared block
Introduces Disabled.Consumer so that components are able to tell if they
are wrapped in a <Disabled> or not. Then, use this to force HTML blocks
to render in preview mode when disabled. This will be the case when in a
shared block.
@gziolo

gziolo approved these changes Jun 7, 2018

It took me a while to understand what kind of magic is happening in this PR. I like this usage of Context API. I must admit it nicely takes advantage of the virtual DOM tree scoping to inform child blocks that they should be in disabled state ❤️

It also tests well, very nice improvement. Can we use it also with other blocks that have preview mode, maybe shortcodes?

@noisysocks noisysocks merged commit d44f116 into master Jun 8, 2018

2 checks passed

codecov/project Absolute coverage decreased by -0.17% but relative coverage increased by +3.6% compared to a2520a3
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@noisysocks noisysocks deleted the add/force-html-block-to-preview-when-shared branch Jun 8, 2018

@noisysocks

This comment has been minimized.

Member

noisysocks commented Jun 8, 2018

Thanks @gziolo! To my knowledge, the HTML block is the only block that has a distinction between edit mode and preview mode when viewed in the visual editor.

@aduth

I really like this approach! 👍

<div ref={ this.bindNode } className="components-disabled">
{ this.props.children }
</div>
<Provider value={ true }>

This comment has been minimized.

@aduth

aduth Jun 8, 2018

Member

In my testing, if we set the default value for context above to true, there's no need to pass value at all.

i.e.

const { Consumer, Provider } = createContext( true );

// ...

return <Provider>{ /* ... */ }</Provider>;

Demo: https://codepen.io/aduth/pen/KeNGjL

@gziolo

This comment has been minimized.

Member

gziolo commented Jun 8, 2018

@aduth, the thing is that you want to default to false so you could override the value with the consumer with true only for selected branches - shared block in edit mode as of this PR.

@aduth

This comment has been minimized.

Member

aduth commented Jun 8, 2018

Oooh right, makes sense. I guess I forgot that the consumer is expected to be used even when a provider ancestor doesn't exist (i.e. detect that element is, in-fact, not disabled).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment