withspectrum / draft-js-markdown-plugin

An opinionated DraftJS plugin for supporting Markdown syntax shortcuts
https://markdown-plugin.spectrum.chat/
MIT License
139 stars 42 forks source link

Inline styles applied with something other than markdown shortcut are lost after a single letter #122

Open Rosey opened 6 years ago

Rosey commented 6 years ago

Just testing this on the spectrum.chat site:

  1. start typing a message
  2. Press cmd-b to make text bold
  3. Continue typing

Expected result: Text is bold from the time you cmd-b until ... forever, unless you press cmd-b again. Actual result: The first letter you type after cmd-b will be bold but all subsequent letters will revert to unstyled.

Rosey commented 6 years ago

I think https://github.com/withspectrum/draft-js-markdown-plugin/commit/935fe755ee3b30c371d55366198efd0555dffbe0 is the culprit, which I'm sure you have already figured out as the author, haha. Probably takes longer for me to sort through everything. I wonder if there's some way to get the best of both worlds here...

Rosey commented 6 years ago

Here's a possible solution that might be a terrible idea but I'll just write it down in case it's not terrible.

Markdown shortcut applied styles are updated to be complex inline styles keyed with markdown-BOLD and are written to behave/appear the same way as BOLD so no one can tell the difference? But then when doing this unstickyInlineStyles we only unsticky if the inline style is a markdown prefixed style?

🙊 oh gosh that's probably too complicated of a solution, hopefully someone else has a smart idea 🤓

mxstbr commented 6 years ago

Huh, I didn't even know you could press CMD+B to make things bold. How do you stop making things bold and where is that shortcut coming from?

mxstbr commented 6 years ago

Markdown shortcut applied styles are updated to be complex inline styles keyed with markdown-BOLD and are written to behave/appear the same way as BOLD so no one can tell the difference? But then when doing this unstickyInlineStyles we only unsticky if the inline style is a markdown prefixed style?

Yeah that could work, would just require a lot of internal hacking around and would probably break integration with some other plugins. 🤔

Maybe we can override CMD+B to not only make things bold but also disable unstickyInlineStyles until you stop typing bold text? (however one does that when pressing CMD+B) Are there other shortcuts like that?

Rosey commented 6 years ago

There's a bunch of shortcuts and then we'll encounter the same issue when pressing toolbar buttons if a user has any kind of tool bar in place :( but I agree my suggestion is too complicated too.

Rosey commented 6 years ago

Re: your question, the cmd b etc is something you have to implement, but draft does make it easy to add https://draftjs.org/docs/api-reference-rich-utils.html RichUtils.handleKeyCommand

Rosey commented 6 years ago

If an elegant solution is tricky to find, in the interim is there any possibility we could add an option to disable the unstickyInlineStyles functionality?


One other possible solution I thought of is always adding an additional markdown-shortcut inline style any time an inline style is applied via this plugin, and unsticky-ing the "normal styles" only IF that inline style is present. BUT I think that would also be bug prone and edge casey :/ Sorry for not having smarter ideas!

andrewfan commented 6 years ago

@Rosey @mxstbr I think the best solution will be to use custom change type here https://github.com/withspectrum/draft-js-markdown-plugin/blob/master/src/modifiers/handleInlineStyle.js#L64 (the place where we convert chars sequence to text with inline style) and unstick styles only if current state was modified with that custom type change.

Will make a PR for it

Kysumi commented 3 years ago

Just run into this problem when using this plugin. Are there any known workarounds or plans to resolve this issue?