Fixes #36 - custom inline styles #97
Conversation
|
Thank you @vincentaudebert 👏 ! Looks good from afar, I'll do a proper review next week. |
lib/api/DraftUtils.js
Outdated
| parseCustomStyleMap(constantInlineStyles, defaultCustomStyleMap, inlineStyles) { | ||
| const customStyleMap = defaultCustomStyleMap; | ||
| const styleValues = Object.keys(constantInlineStyles).map(k => constantInlineStyles[k]); | ||
| const verifiedStyles = inlineStyles.filter(style => styleValues.indexOf(style.type) !== -1); |
There was a problem hiding this comment.
This seems problematic. If the type isn't in constantInlineStyles, it's not going to be in verifiedStyles and won't be added to the style map, making it impossible to define styles for custom inline styles.
lib/api/constants.js
Outdated
| export const CUSTOM_STYLE_MAP = { | ||
| [INLINE_STYLE.MARK]: { | ||
| backgroundColor: 'yellow', | ||
| color: 'black', |
There was a problem hiding this comment.
Will remove the color override – I think it would be better if we retained the normal text color.
lib/api/constants.js
Outdated
| marginLeft: '1em', | ||
| marginTop: '1em', | ||
| marginBottom: '1em', | ||
| }, |
There was a problem hiding this comment.
This looks like the styles of a blockquote. Here we should target inline styles only, so this should be the q element: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/q.
lib/components/DraftailEditor.js
Outdated
| @@ -49,6 +49,7 @@ const defaultProps = { | |||
| maxListNesting: 1, | |||
| // Frequency at which the save callback is triggered (ms). | |||
| stateSaveInterval: 250, | |||
| customStyleMap: {}, | |||
There was a problem hiding this comment.
This seems like a leftover from a previous iteration.
lib/components/DraftailEditor.js
Outdated
| @@ -77,6 +78,7 @@ const propTypes = { | |||
| type: PropTypes.string.isRequired, | |||
| // Represents the inline style in the editor UI. | |||
| icon: PropTypes.string, | |||
| style: PropTypes.Object, | |||
There was a problem hiding this comment.
Every single one of these props must be commented so we can add those comments in the README as documentation.
lib/components/DraftailEditor.js
Outdated
| const decorators = entityTypes.filter(type => !!type.decorator) | ||
| .map(type => ({ | ||
| strategy: type.strategy || DraftUtils.getEntityTypeStrategy(type.type), | ||
| component: type.decorator, | ||
| })); | ||
|
|
||
| const customStyleMapParsed = DraftUtils.parseCustomStyleMap(INLINE_STYLE, CUSTOM_STYLE_MAP, inlineStyles); |
There was a problem hiding this comment.
Wouldn't it be simpler if this was directly done in render, instead of being added to the state? Here this will fall out of date if the editor is re-rendered with new values for inlineStyles.
thibaudcolas
force-pushed
the
feature/custom-inline-styles
branch
2 times, most recently
from
1e08cfe to
79810c3
Compare
| @@ -108,7 +108,7 @@ Draftail, like Draft.js, distinguishes between 3 content formats: | |||
| Common formatting options are available out of the box: | |||
|
|
|||
| - Block types: `H1`, `H2`, `H3`, `H4`, `H5`, `H6`, `Blockquote`, `Code`, `UL`, `OL`, `P` | |||
| - Inline styles: `Bold`, `Italic`, `Underline`, `Monospace`, `Strikethrough` | |||
| - Inline styles: `Bold`, `Italic`, `Code`, `Underline`, `Strikethrough`, `Mark`, `Keyboard`, `Superscript`, `Subscript` | |||
There was a problem hiding this comment.
There are more, but I chose to leave them out because they are seldom used in rich text editors.
|
|
||
| Simple blocks are very easy to create. Add a new block type to `blockTypes`, specifying which `element` and `className` to display the block as. | ||
|
|
||
| Here is an example, creating a "Terms & conditions" block: |
There was a problem hiding this comment.
It's not my fav example but I hope it's easy enough to relate to the need for this.
| More advanced blocks, requiring custom React components, aren't supported at the moment. If you need this, feel free to [create an issue](https://github.com/springload/draftail/issues/new). | ||
|
|
||
| #### Custom inline styles | ||
|
|
||
| Custom inline styles aren't supported at the moment. This is on the roadmap, please refer to [#36](https://github.com/springload/draftail/issues/36). | ||
| Custom inline styles only require a `style` prop, defining which CSS properties to apply when the format is active. |
There was a problem hiding this comment.
It's not actually required – but if it's not there, there will be no way to tell whether the styles are active or not.
Fixes #36. For your review @thibaudcolas
Added a new draftUtils function and tested it with some edge cases + 100% coverage.
Let me know what you think.