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

Fix toolbar buttons appearing broken when no label or icon is set #442

Merged
merged 4 commits into from
Apr 13, 2021

Conversation

thibaudcolas
Copy link
Collaborator

@thibaudcolas thibaudcolas commented Apr 13, 2021

This PR changes two things:

  • Custom formats without a defined label or icon are now hidden from the toolbar. Those two props have always been optional, but having neither really doesn’t achieve anything useful for users. This also makes it easier to implement custom controls (e.g. color picker for inline styles, or text styles drawer).
  • Buttons for built-in formats like this can also be hidden by explicitly setting their label to null (thus overriding the built-in labels).

I also added a demo of building a text styles drawer as part of the "controls" doc, which depends on those changes.


  • Stay on point and keep it small so it can be easily reviewed. For example, try to apply any general refactoring separately outside of the PR.
  • Consider adding unit tests, especially for bug fixes. If you don't, tell us why.
  • All new and existing tests pass, with 100% test coverage (npm run test:coverage)
  • Linting passes (npm run lint)
  • Consider updating documentation. If you don't, tell us why.
  • List the environments / platforms in which you tested your changes.

Thanks for contributing to Draftail!

@thibaudcolas thibaudcolas force-pushed the bug/empty-toolbar-buttons branch from 2c61f54 to 080a64a Compare April 13, 2021 07:19
@github-actions
Copy link

Warnings
⚠️ This pull request updates the library. Should the documentation be updated as well?
Messages
📖 What labels should we add to this PR?

Generated by 🚫 dangerJS against 080a64a

@thibaudcolas thibaudcolas added the enhancement New feature or request label Apr 13, 2021
@thibaudcolas thibaudcolas added this to the v1.4.0 milestone Apr 13, 2021
@thibaudcolas thibaudcolas marked this pull request as ready for review April 13, 2021 07:35
@github-actions
Copy link

Warnings
⚠️ This pull request updates the library. Should the documentation be updated as well?

Generated by 🚫 dangerJS against c39bf8a

Copy link
Contributor

@emilytoppm emilytoppm left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for making this change!

@thibaudcolas thibaudcolas merged commit 4b7fbc1 into main Apr 13, 2021
@thibaudcolas thibaudcolas deleted the bug/empty-toolbar-buttons branch April 13, 2021 13:56
@thibaudcolas thibaudcolas mentioned this pull request Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants