Unsticky inline styles #83

mxstbr · 2018-05-24T07:57:41Z

This "unstickies" inline styles, so that when you type bold text, backspace to the ending edge of the bold text and start typing again the bold doesn't "stick" to the text, only when typing within it. This feels much better than before.

Recorded demo:
Live demo: https://public-mffnxzocik.now.sh

/cc @brianlovin @wmertens this should be a fix for the issues you were having!

This "unstickies" inline styles, so that when you type *bold* text, backspace to the ending edge of the bold text and start typing again the bold doesn't "stick" to the text, only when typing within it. This feels much better than before. Recorded demo: ![](https://i.imgur.com/gWSLp06.gif)

juliankrispel

Niiiiiiiiiiiice! Just one comment and maybe some unit tests?

juliankrispel · 2018-05-24T08:15:48Z

src/index.js

+  const content = editorState.getCurrentContent();
+  const block = content.getBlockForKey(selection.getStartKey());
+  const entity = block.getEntityAt(startOffset);
+  if (entity !== null) return editorState;


It'd be nice if you could also add a check for an inline entity

How do I do that?

hold on actually you're already doing that with block.getEntityAt - my bad :D

juliankrispel · 2018-05-24T08:18:22Z

src/index.js

+      if (editorState !== unsticky) {
+        setEditorState(unsticky);
+        return "handled";
+      }


would be nice to add a bunch of unit tests for this 👍

Unit test added in latest commit!

thibaudcolas · 2018-05-24T09:57:12Z

This looks pretty cool! Do you use it in an editor that also has normal keyboard shortcuts or toolbar buttons for inline styles? While for Markdown shortcuts I think it makes perfect sense, I wonder how end users like it if they trigger bold with the usual Ctrl + B, or a button.

mxstbr · 2018-05-24T10:03:48Z

@thibaudcolas yes, this is just a small piece in our overall editing experience—we're also working on adding an inline toolbar to trigger those inline styles.

juliankrispel · 2018-05-24T10:05:35Z

src/__test__/plugin.test.js

+              expect(block.getInlineStyleAt(length - 2).toJS()).toEqual([
+                "BOLD",
+              ]);
+            });


I'd also add some unit tests for when stickiness doesn't apply: like when you're in an entity for example 👍 just to make sure it doesn't break in future

I tried for like an hour to write another test, but it always has the wrong content state (text: altered!!) and I don't know where it's coming from or how to fix it 😢 Any clues?

wmertens · 2018-05-24T13:20:39Z

I don't know, I would really prefer that the invisible markers become visible again. If I type `some cod` and realize the `e` is missing, I would have to go before the `d`, type `de` and then remove the ending `d`. If instead backspacing deletes the invisible ` character, it would be obvious how to fix the text.

…

On Thu, May 24, 2018 at 2:24 PM Max Stoiber ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/__test__/plugin.test.js <#83 (comment)> : > + + currentSelectionState = currentSelectionState.merge({ + focusOffset: 5, + anchorOffset: 5, + }); + + expect(subject()).toBe("handled"); + expect(store.setEditorState).toHaveBeenCalled(); + newEditorState = store.setEditorState.mock.calls[0][0]; + const block = newEditorState.getCurrentContent().getLastBlock(); + const length = block.getLength(); + expect(block.getInlineStyleAt(length - 1).toJS()).toEqual([]); + expect(block.getInlineStyleAt(length - 2).toJS()).toEqual([ + "BOLD", + ]); + }); I tried for like an hour to write another test, but it always has the wrong content state (text: altered!!) and I don't know where it's coming from or how to fix it 😢 Any clues? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#83 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADWliqDpDxGuEiuVLic3Q-keKD0JYlhks5t1qZogaJpZM4ULvcP> .

mxstbr · 2018-05-24T13:24:59Z

If instead backspacing deletes the invisible ` character, it would be obvious how to fix the text.

It's not invisible though, it's not there anymore—so it's not as simple as just removing that! I totally see why pressing backspace at the end removing the inline styling would be nice, will see if we can tackle that in the future sometime, doesn't contradict this change though!

juliankrispel · 2018-05-24T13:31:43Z

src/index.js

+  const startOffset = selection.getStartOffset();
+  const content = editorState.getCurrentContent();
+  const block = content.getBlockForKey(selection.getStartKey());
+  const entity = block.getEntityAt(startOffset);


this needs to be startOffset - 1 btw, just as below

mxstbr · 2018-05-24T13:38:53Z

Great catch, done

wmertens · 2018-05-24T14:51:54Z

How about adding a right-hand border on the current range, and allowing the cursor to be to the left or to the right of the border, even though the border is not an actual character? Is that possible? Then you can easily choose to add to the end or not…

…

On Thu, May 24, 2018 at 3:39 PM Max Stoiber ***@***.***> wrote: Great catch, done — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#83 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADWlpvZffCOrLsOzuHOK9mZoMZ4HAynks5t1rf1gaJpZM4ULvcP> .

mxstbr · 2018-05-24T15:43:06Z

Probably possible somehow, but again a lot more work and doesn't really have an impact on this change 😉

Will do in a follow-up PR with a new test harness

mxstbr mentioned this pull request May 24, 2018

Need ability to "not continue" an inline style or mutable entity facebookarchive/draft-js#430

Open

juliankrispel previously requested changes May 24, 2018

View reviewed changes

Add unit test

ddc0488

juliankrispel suggested changes May 24, 2018

View reviewed changes

juliankrispel previously requested changes May 24, 2018

View reviewed changes

Fix entity check

2a98941

mxstbr merged commit 65fee79 into master May 24, 2018

mxstbr deleted the unsticky branch May 24, 2018 16:36

thibaudcolas mentioned this pull request May 30, 2018

Add Markdown-style shortcuts for inline formats springload/draftail#134

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unsticky inline styles #83

Unsticky inline styles #83

mxstbr commented May 24, 2018

juliankrispel left a comment

juliankrispel May 24, 2018

mxstbr May 24, 2018

juliankrispel May 24, 2018

juliankrispel May 24, 2018

thibaudcolas commented May 24, 2018 •

edited

Loading

mxstbr commented May 24, 2018

juliankrispel May 24, 2018

mxstbr May 24, 2018

wmertens commented May 24, 2018 via email

mxstbr commented May 24, 2018 •

edited

Loading

juliankrispel May 24, 2018

mxstbr commented May 24, 2018

wmertens commented May 24, 2018 via email

mxstbr commented May 24, 2018

Unsticky inline styles #83

Unsticky inline styles #83

Conversation

mxstbr commented May 24, 2018

juliankrispel left a comment

Choose a reason for hiding this comment

juliankrispel May 24, 2018

Choose a reason for hiding this comment

mxstbr May 24, 2018

Choose a reason for hiding this comment

juliankrispel May 24, 2018

Choose a reason for hiding this comment

juliankrispel May 24, 2018

Choose a reason for hiding this comment

thibaudcolas commented May 24, 2018 • edited Loading

mxstbr commented May 24, 2018

juliankrispel May 24, 2018

Choose a reason for hiding this comment

mxstbr May 24, 2018

Choose a reason for hiding this comment

wmertens commented May 24, 2018 via email

mxstbr commented May 24, 2018 • edited Loading

juliankrispel May 24, 2018

Choose a reason for hiding this comment

mxstbr commented May 24, 2018

wmertens commented May 24, 2018 via email

mxstbr commented May 24, 2018

thibaudcolas commented May 24, 2018 •

edited

Loading

mxstbr commented May 24, 2018 •

edited

Loading