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

Add markdown editor toolbar customization #1236

Merged
merged 4 commits into from
Apr 16, 2018

Conversation

Dammmien
Copy link
Contributor

@Dammmien Dammmien commented Apr 6, 2018

Add markdown editor toolbar customization #1194

- Summary

Closes #1194

Add markdown toolbar buttons customization, based on conf file :

- ...
- {label: "Markdown", name: "markdown", widget: "markdown", buttons: ["bold", "italic", "link"]}
- ...

- Test plan

If no buttons field is specified we display all the buttons otherwise we display the list.

- Description for the changelog

Add markdown editor toolbar customization

@verythorough
Copy link
Contributor

verythorough commented Apr 6, 2018

Deploy preview for cms-demo ready!

Built with commit 73630a4

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

@verythorough
Copy link
Contributor

verythorough commented Apr 6, 2018

Deploy preview for netlify-cms-www ready!

Built with commit 73630a4

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

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 for this @Dammmien! Couple of thoughts for you in the review.

const {
onMarkClick,
onBlockClick,
onLinkClick,
selectionHasMark,
selectionHasBlock,
selectionHasLink,
disabled,
Copy link
Contributor

Choose a reason for hiding this comment

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

We could minimize changes to this file by simply adding a hidden prop to ToolbarButton. What do you think?

Copy link
Contributor Author

@Dammmien Dammmien Apr 12, 2018

Choose a reason for hiding this comment

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

And if the the hidden property is true, the render function simply return null ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep. I'd usually avoid components managing their own visibility, but in this case it feels worth the trade off.

@@ -14,6 +14,7 @@ The markdown widget provides a full fledged text editor - which is based on [sla
- **Data type:** markdown
- **Options:**
- `default`: accepts markdown content
- `buttons`: a list of string representing the buttons to display ('bold', 'italic', 'code', 'link', 'heading-one', 'heading-two', 'quote', 'code', 'bulleted-list', 'numbered-list'). If this options is missing, all the buttons are display.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update to:

  • buttons: an array of strings representing the formatting buttons to display, all buttons shown by default. Buttons include: "bold", "italic", "code", "link", "heading-one", "heading-two", "quote", "code-block", "bulleted-list", and "numbered-list".

While we happen to be using the types from our code, we shouldn't rely on this option aligning with type names. Note that I changed the second "code" to "code-block".

@Dammmien
Copy link
Contributor Author

@erquhart I added the isHidden property, and indeed it minimize changes.

@erquhart
Copy link
Contributor

Awesome! Can you update the docs snippet to this edited version:

  • buttons: an array of strings representing the formatting buttons to display, all buttons shown by default. Buttons include: bold, italic, code, link, heading-one, heading-two, quote, code-block, bulleted-list, and numbered-list.

@Dammmien
Copy link
Contributor Author

@erquhart documentation updated

@erquhart erquhart merged commit 32a340e into decaporg:master Apr 16, 2018
@Dammmien Dammmien deleted the 1194_customize_markdown_editor branch April 26, 2018 20:15
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.

None yet

4 participants