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

feat(react-slate): Ability to add multiple placeholders. #4187

Closed
wants to merge 2 commits into from
Closed

feat(react-slate): Ability to add multiple placeholders. #4187

wants to merge 2 commits into from

Conversation

xobotyi
Copy link

@xobotyi xobotyi commented Apr 10, 2021

Description
As for my case i had a need to add multiple placeholders for a single editable, that are hidden separately as content fills.

Example
123
Above example has such placeholder property:

placeholder={[
  'Summarize your issue.',
  <>
    And describe it in details here.
    <br />
    Try to avoid using languages not supported by interface, it will significantly improve
    and speed up our work.
  </>,
]}

Checks

  • The new code matches the existing patterns and styles.
  • The tests pass with yarn test.
  • The linter passes with yarn lint. (Fix errors with yarn fix.)
  • The relevant examples still work. (Run examples with yarn start.)
  • You've added a changeset if changing functionality. (Add one with yarn changeset add.)

- Now previews can be any of ReactNode or ReactNodeArray
- Leaf component is fully refactored, no effects used anymore, just
straight render.
- Preview styling moved to `DefaultLeaf` component so users now able to
change it on their need via `Editable.renderLeaf`.
- `Editable` component has proper typing now.

BREAKING CHANGE: anyone using `Editable.renderLeaf` and not calling
`DefaultLeaf` on placeholder leaves wont have placeholder displayed.
Sadly this is the only way to allow placeholder customisation.
Each of them will create own empty space and will be hidden separately
when it's empty place filled.

Also, instead of adding new line having placeholder ahead editor will
simply switch to next empty space.
@changeset-bot
Copy link

changeset-bot bot commented Apr 10, 2021

🦋 Changeset detected

Latest commit: af331fb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
slate-react Minor
slate Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ianstormtaylor
Copy link
Owner

@xobotyi can you see if your use case would be solved by #4190 with fragment rendering? It seems like a simpler solution so I'd rather go with it if possible.

@xobotyi
Copy link
Author

xobotyi commented Apr 13, 2021

@ianstormtaylor current placeholder will disappead at the moment first character entered. Thus no, it cant be achieved with renderer.

@ianstormtaylor
Copy link
Owner

@xobotyi I believe your case could instead be solved by handling the placeholder logic at the block level? One for the heading and one for the paragraph like with the Forced Layout example?

@xobotyi
Copy link
Author

xobotyi commented Apr 13, 2021

@ianstormtaylor and no again, at block level i wont be able to gain access to keypress handlers to move between placholders by pressing enter.
Currently if you press Enter and there is empty placeholder ahead, it will switch to that line instead of splitting current one. I dont know how to manage it outside the slate.

@ianstormtaylor
Copy link
Owner

@xobotyi I'm not sure I understand what you mean by splitting the block. But since placeholders internally are implemented with decorations, I believe you could implement a similar behavior in userland with special decoration logic for your two nodes.

@xobotyi
Copy link
Author

xobotyi commented Apr 13, 2021

@ianstormtaylor when enter pressed - handler calls Transforms.splitNodes which splits current line in two (i did not told about block splitting).
If such question was only about decoration i wold not change the base components, here it is also about behaviour

Copy link
Collaborator

@dylans dylans left a comment

Choose a reason for hiding this comment

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

@xobotyi unfortunately this one has fallen a bit behind main.

If you're still interested in landing this, please update against the latest and then I'll review. Apologies that this PR was ignored after your last comment.

@dylans
Copy link
Collaborator

dylans commented Sep 7, 2021

Closing this one out due to inactivity and it being stale. Feel free to reopen or create a new PR if I missed something here. Thanks!

@dylans dylans closed this Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants