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

Can you allow for the readOnly prop to be passed to the underlying Editor? #201

Closed
callmeberzerker opened this issue Jun 26, 2019 · 7 comments
Labels
enhancement New feature or request
Milestone

Comments

@callmeberzerker
Copy link
Contributor

Do you want to request a feature or report a bug?
Feature request.
What is the current behavior?
The readOnly property that draft-js Editor allows for, is always set to false.
If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. GIFs and screenshots are very helpful too.

What is the expected behavior?
Pass readOnly to the draft-js Editor.

@thibaudcolas
Copy link
Collaborator

Hey @SpearThruster 👋 can you explain what you would find this useful for? At the moment readOnly can already be set to true, via the toggleEditor function usually used by the entity source API. I can see why it would be useful to set the editor to read-only, just not sure why it’s useful to have this as a prop instead of the imperative API.

@thibaudcolas thibaudcolas added the enhancement New feature or request label Jul 22, 2019
@thibaudcolas thibaudcolas added this to the Nice to have milestone Jul 22, 2019
@callmeberzerker
Copy link
Contributor Author

Hi @thibaudcolas,

I think it's a straight forward use-case, where a certain user can edit the RTE field while other's can't. IMO a prop makes sense for such a functionality.

@callmeberzerker
Copy link
Contributor Author

Hi @thibaudcolas

This is a pretty straight forward backwards compatibly change, that's supported by draft-js -> I can provide a PR if you'd merge it. :)

@thibaudcolas
Copy link
Collaborator

Yeah I see, it does make sense to support this from a Draft.js compatibility perspective. I’d gladly accept a PR for this then!

I think the only thing to watch out for is that readOnly on the Draft.js editor is already set based on the editor’s state, also called readOnly – so will need to rename that state to something else, and have readOnly as the prop’s name. Then use these two in the editor rendering, something like:

  render() {
    const {
      [...]
      readOnly,
      [...]
    } = this.props;
    const { editorState, hasFocus, readOnlyState } = this.state;
    const isReadOnly = readOnlyState || readOnly;

    return (
      <div
        className={`Draftail-Editor${
          isReadOnly ? " Draftail-Editor--readonly" : ""
        }${hidePlaceholder ? " Draftail-Editor--hide-placeholder" : ""}${
          hasFocus ? " Draftail-Editor--focus" : ""
        }`}
      >
        {TopToolbar ? <TopToolbar {...toolbarProps} /> : null}

        <Editor
          [...]
          readOnly={isReadOnly}
          [...]

Oh and we’ll also need to add this prop to https://www.draftail.org/docs/api, although I can take care of that when releasing the changes.

@callmeberzerker
Copy link
Contributor Author

PR opened. :)

@thibaudcolas
Copy link
Collaborator

Added in #206!

@thibaudcolas thibaudcolas modified the milestones: Nice to have, v1.3.0 Aug 7, 2019
@thibaudcolas
Copy link
Collaborator

This is now released in Draftail v1.3.0 – https://www.draftail.org/blog/2019/08/15/draftail-v1-3-0-community-improvements-beyond-wagtail 🎉

Thanks again for the help @SpearThruster.

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

No branches or pull requests

2 participants