accordproject / web-components

React Components for Accord Project
Apache License 2.0
119 stars 99 forks source link

fix(ui-markdown-editor): allow image deletion - #320 #330

Closed K-Kumar-01 closed 3 years ago

K-Kumar-01 commented 3 years ago

Signed-off-by: k-kumar-01 kushalkumargupta4@gmail.com

Closes #320

Image can be deleted now by pressing either the backspace key or the delete key.

Changes

Screenshots or Video

Author Checklist

K-Kumar-01 commented 3 years ago

@irmerk Requesting your review.

Aniruddha-Shriwant commented 3 years ago

This looks great @K-Kumar-01! But I just noticed a small bug, Steps to reproduce :

  1. Go to https://deploy-preview-330--ap-web-components.netlify.app/?path=/story/markdown-editor--demo
  2. Select the default Image
  3. Hit backspace, Now you will notice that the image was deleted successfully
  4. Hit ctrl+z or undo twice, the image is back again
  5. Now select the image and hit backspace again, you will notice that the last letter of the line above the image is also deleted

@K-Kumar-01 can you verify this? Is this a bug or it's intentional to work like that?

K-Kumar-01 commented 3 years ago

This looks great @K-Kumar-01! But I just noticed a small bug, Steps to reproduce :

  1. Go to https://deploy-preview-330--ap-web-components.netlify.app/?path=/story/markdown-editor--demo
  2. Select the default Image
  3. Hit backspace, Now you will notice that the image was deleted successfully
  4. Hit ctrl+z or undo twice, the image is back again
  5. Now select the image and hit backspace again, you will notice that the last letter of the line above the image is also deleted

@K-Kumar-01 can you verify this? Is this a bug or it's intentional to work like that?

@Aniruddha-Shriwant I found this while I was trying to resolve the issue. However, I left it as such because I didn't think that anyone would do the following steps: delete->undo->delete. So I kept it as such. If there is any change required, I would do it.

Aniruddha-Shriwant commented 3 years ago

Yeah @K-Kumar-01 that makes sense 😅