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

Filters should inject blocks before and after atomic blocks as required #14

Open
thibaudcolas opened this issue Mar 7, 2018 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Milestone

Comments

@thibaudcolas
Copy link
Owner

thibaudcolas commented Mar 7, 2018

Feature (to address a bug in Draft.js). See facebookarchive/draft-js#523 (comment) and facebookarchive/draft-js#1082.

What is the current behavior?

[...] when copy-pasting Draft.js images from one editor to another. If the unstyled blocks in-between the images are empty, then they will be removed in the target editor... and it becomes impossible to select the images to remove them or add content before/after.

This also happens for other block types than unstyled – all that's needed is for the blocks to be empty.

What is the expected behavior?

Blocks should be preserved. The filters can't preserve the right block type (would require a fix in Draft.js), but they could at least inject unstyled blocks where appropriate. There are three cases I know of:

  • The image is the first block, so it needs to be preceded with an empty block.
  • The image is the last block, so it needs to be followed by an empty block.
  • Two images one after the other need to have an empty block in-between.

For the implementation, I think this should probably go after removeInvalidAtomicBlocks in filterEditorState, so it only happens where really necessary.

I imagine the implementation would need to reduce through the blocks, inserting new ones where appropriate. This probably needs to happen for atomic blocks rather than just images, but AFAIK only images are preserved on copy-paste anyway, at least for now.

@thibaudcolas thibaudcolas added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Mar 7, 2018
@thibaudcolas thibaudcolas added this to the v2.0.0 milestone Mar 7, 2018
@thibaudcolas
Copy link
Owner Author

Analysing this further,

This can happen as the last stage of filterEditorState, since there is no point in filtering the added blocks.

// 8. Finally, do text operations.
replaceTextBySpaces.bind(null, whitespacedCharacters),
]

This might conflict with the current mode of operation of HORIZONTAL_RULE blocks in Draftail, which are created while replacing the current line without the spacer blocks – if we auto-add those spacers on paste, it would add them within this manually-entered content.

@thibaudcolas
Copy link
Owner Author

I'm not sure if this feature would make much sense with editors like Draftail using thibaudcolas/draftjs-conductor#2. It probably makes sense to make it configurable per entity type at least, considering things like springload/draftail#158.

@thibaudcolas
Copy link
Owner Author

thibaudcolas commented Jan 23, 2019

Note for future self: this is valuable not just for atomic blocks, but for any block type that is rendered as editable: false and does not implement custom means of selection. See springload/draftail#169 as an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant