Skip to content
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

Don't render hidden widgets in markdown visual editor #1414

Merged

Conversation

robertkarlsson
Copy link

@robertkarlsson robertkarlsson commented Jun 8, 2018

Fixes #1405

- Summary

Fields using hidden was rendered in the markdown editor with "no control" error if no label attached, else a grey box. Return null when rendering the control for hidden widgets.

- Test plan
After change UI no longer displays hidden components in the markdown visual editor, nor the markdown editor.
image

image

This was tested using the registered widget in the bug report, attached below.

CMS.registerEditorComponent({
  // Internal id of the component
  id: "youtube",
  // Visible label
  label: "Youtube",
  // Fields the user need to fill out when adding an instance of the component
  fields: [{
    name: 'id',
    label: 'Youtube Video ID',
    widget: 'string'
  }, {
    name: 'myHiddenField',
    label: 'My hidden field',
    widget: 'hidden'
  }],
  // Pattern to identify a block as being an instance of this component
  pattern: /^youtube (\S+)$/,
  // Function to extract data elements from the regexp match
  fromBlock: function(match) {
    return {
      id: match[1]
    };
  },
  // Function to create a text block from an instance of this component
  toBlock: function(obj) {
    return 'youtube ' + obj.id;
  },
  // Preview output for this component. Can either be a string or a React component
  // (component gives better render performance)
  toPreview: function(obj) {
    return (
      '<img src="http://img.youtube.com/vi/' + obj.id + '/maxresdefault.jpg" alt="Youtube Video"/>'
    );
  }
});

- Description for the changelog
Return null when rendering hidden widgets in the markdown editor.

- A picture of a cute animal (not mandatory but encouraged)
image

Fixes decaporg#1405
Fields using hidden was rendered in the markdown editor with "no control" error if no label attached, else a grey box.
Return null when rendering the control for hidden widgets.
@verythorough
Copy link
Contributor

verythorough commented Jun 8, 2018

Deploy preview for cms-demo ready!

Built with commit 2e270c6

https://deploy-preview-1414--cms-demo.netlify.com

@verythorough
Copy link
Contributor

verythorough commented Jun 8, 2018

Deploy preview for netlify-cms-www ready!

Built with commit 2e270c6

https://deploy-preview-1414--netlify-cms-www.netlify.com

@suessflorian
Copy link

suessflorian commented Jun 8, 2018

Hey! To throw in my salt; another approach could be to construct a boolean holding if this widget is hidden or not.

const hidden = field.get('widget') === 'hidden' ? 'display: hidden;' : 'display: block;'

then on the div upon return;
div key={key} style={hidden}

@robertkarlsson
Copy link
Author

robertkarlsson commented Jun 9, 2018

@fellasuess It makes sense and i thought about this as well, then i found the following code for other hidden widgets:

https://github.com/netlify/netlify-cms/blob/78e4b829d0ced72f8f38721d38c0d67ccb092aaf/src/components/Editor/EditorControlPane/EditorControlPane.js#L47

Maybe the preferred way is to not execute the rest of the code (by the early return of null) if the component has no value of being rendered. Anyone else has an opinion on this?

@erquhart
Copy link
Contributor

Rendering null is 👍

Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

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

Thanks!

@erquhart erquhart merged commit d948845 into decaporg:master Jun 12, 2018
erquhart pushed a commit to erquhart/netlify-cms that referenced this pull request Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants