draft-js-code icon indicating copy to clipboard operation
draft-js-code copied to clipboard

Protection against current code-block removing

Open newsiberian opened this issue 7 years ago • 7 comments

Hi, this should go after previous PRs. We don't need getIndentation.js anymore.

When we are at the beginning of line at our code-block we can remove it by making attempt to go back to previous indent. Here we are checking the current indent and if it zero and this is the first line of the code-block, we do nothing.

newsiberian avatar Dec 07 '17 04:12 newsiberian

Coverage Status

Coverage decreased (-12.2%) to 87.075% when pulling 6d75d13d8da61f47eb7fdeb6b414ac724140f00d on newsiberian:protect-against-code-block-removing into c219dc50d42f7db9c69b4b8c9be4b437116e8e6a on SamyPesse:master.

coveralls avatar Dec 07 '17 04:12 coveralls

If I understand this correctly, when you're in an empty code block with the cursor at the first line, i.e. like so: (where | is the cursor position)

|

Rather than trying to remove the (non-existant) indentation we just let DraftJS handle it instead? What's not working about this right now?

mxstbr avatar Dec 07 '17 15:12 mxstbr

You can try it here with backspace If you try to put cursor into position you mentioned (even if text presents in code-block) and press backspace it will remove code-block.

newsiberian avatar Dec 07 '17 16:12 newsiberian

Ohhh I see, that's annoying, great catch! Let's get some tests in here to make sure this doesn't happen again accidentally, and then let's ship it :rocket:

mxstbr avatar Dec 07 '17 16:12 mxstbr

I'll try to add tests tomorrow

newsiberian avatar Dec 07 '17 16:12 newsiberian

Awesome, thank you! There's a bunch of tests already, you can just model them after that 👌

mxstbr avatar Dec 07 '17 16:12 mxstbr

Coverage Status

Coverage decreased (-0.6%) to 98.701% when pulling d4796f778e4bb22f07e758ef18bac06d01f2ca04 on newsiberian:protect-against-code-block-removing into c219dc50d42f7db9c69b4b8c9be4b437116e8e6a on SamyPesse:master.

coveralls avatar Dec 11 '17 06:12 coveralls