Add prop to optionally enable grammarly

[nadeemshaik]

Is this adding or improving a feature or fixing a bug?

Feature(i guess): This enables the user to optionally pass a prop to enable Grammarly which was previously disabled in #733.

What's the new behavior?

You can also check it here sandbox. You will, of course, need to install the Grammarly extension. The above demo was tested on version 14.915.2321

How does this change work?

Since Grammarly has moved to an API based content modification, it would be really helpful for users to be able to enable Grammarly if it suits them. So I have added a prop to enable the same. The only non-trivial part is that the extension doesn't seem to work as long as the prop data-gramm exists irrespective of its value.

Have you checked that...?

• 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 prettier.)
• The relevant examples still work. (Run examples with yarn watch.)

Does this fix any issues or need any specific reviewers?

Fixes: #733
Reviewers:

[isubasti]

LGTM

[brendancarney]

Thanks for the PR @nadeemshaik. I don't think we should have a prop specific to a third party service. Maybe there's a solution that allows passing of generic data attributes to the DOM node?

[nadeemshaik]

Makes sense @brendancarney. There doesn't seem to be existing support for data/dom attributes, but I can see that data attributes are being used locally here Maybe extend this through props?

[brendancarney]

My opinion is that any prop passed to Editor that is not slate specific should be passed through to the dom. So you would do:

<Editor
   {...editorProp}
   data-gram="false"
   data-whatever="true"
/>

and it would work.

This one is probably @ianstormtaylor's call though, so let's see what he thinks.

[ianstormtaylor]

Passthrough for all seems reasonable to me! I agree that it would be good to find a solution that doesn't special-case one prop over others.

[nadeemshaik]

@ianstormtaylor @brendancarney how does this look, couldn't think of a better way to omit the editor/content props. Any ideas?

[nadeemshaik]

Hey @ianstormtaylor, did you get a chance to take a look at this?

[ianstormtaylor]

Thanks @nadeemshaik!

[Jacfem]

Hi @nadeemshaik + @ianstormtaylor! Do you happen to know what slate release included this change? I searched through some recent releases but still see data-gramm set to false. I know Grammarly is a challenge to integrate with many editors, and I'm just trying to understand the state of things in regards to Slate <> Grammarly as it seems to be working on the Slate Demo. Some context is that my team is on an older version of slate (0.34.7) and while investigating what a major upgrade will entail, we'd like to understand if the Grammarly integration might be fixed in later versions. Thanks for any help!

[nadeemshaik]

Hey @Jacfem , I am sure it is present in version 0.44.9. It might have been published just a little earlier than that.
After the fill and till v0.47 , u can do data-gramm={false} and it should work. But after v0.5x major updates were made to the code. With those versions I guess u will need to do something like data-gramm={undefined} explicitly to disable it.

The idea was always to just have the ability to pass any attributes/props from the implementation level and have it override the props of the editor.