Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Requires upgrade to CKEditor5] Spaces above images in the RTE should be deletable. #6225

Open
seanlip opened this issue Feb 7, 2019 · 7 comments
Labels
frontend Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. needs debugging Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@seanlip
Copy link
Member

seanlip commented Feb 7, 2019

Describe the bug
In RTEs, it seems impossible to delete the large vertical space above an image.

To Reproduce
Steps to reproduce the behavior:

  1. Open the exploration editor.
  2. Edit the content field to contain some text, followed by an image, followed by some text.
  3. Delete the text before the image.
  4. Try to delete the whitespace before the image.

Observed behavior
On pressing the Delete key, focus shifts to the image and the vertical space is not deleted.

Expected behavior
The vertical space above the image is deleted.

Screenshots

Before pressing Delete key:
screenshot from 2019-02-06 22-56-14

After pressing Delete key:
screenshot from 2019-02-06 22-56-22

Desktop (please complete the following information; delete this section if the issue does not arise on desktop):

  • OS: Ubuntu Linux
  • Browser: Chromium
  • Version: 71.0
@seanlip
Copy link
Member Author

seanlip commented Feb 7, 2019

/cc @bansalnitish

@bansalnitish
Copy link
Contributor

Thanks for reporting this issue @seanlip . I'll try to fix this.

@bansalnitish bansalnitish self-assigned this Feb 7, 2019
@aks681 aks681 added this to To do in Bug fixing team via automation Feb 10, 2019
@aks681 aks681 moved this from To do to In progress in Bug fixing team Feb 10, 2019
@bansalnitish
Copy link
Contributor

I investigated and found that the issue is reproducible in CKEditor - 4.9.2 itself. The link for its demo -- https://ckeditor.com/ckeditor-4/demo/.

Though, it has been fixed in CKEditor5 :). The link for its demo -- https://ckeditor.com/ckeditor-5/demo/.

So, we need to upgrade CKEditor4 to 5th version. /cc @seanlip

@seanlip
Copy link
Member Author

seanlip commented Feb 22, 2019

Makes sense, thanks. How hard would it be to do that -- is it as straightforward as changing the link in manifest.json? Are there instructions somewhere?

(Also, do you remember why we didn't use CKEditor5 when we upgraded the RTE? Was that because it wasn't out yet at the time, or was there some other reason? /cc @AllanYangZhou )

@bansalnitish
Copy link
Contributor

No, it isn't this straightforward I guess. I tried doing that but I was getting error like:
CKEDITOR is not defined in app.js. I'll have to dig a bit deeper. Here is the complete documentation for the updgradation: https://ckeditor.com/docs/ckeditor5/latest/builds/guides/migrate.html.

I'm not sure as to why we didn't use CKEditor5.

Thanks!

@seanlip
Copy link
Member Author

seanlip commented Feb 23, 2019

Chatted with @bansalnitish. He will have a go at updating to CKEditor5 to see whether any issues come up, and update this issue with the comments.

@bansalnitish
Copy link
Contributor

Assigning this to @import-keshav, he's working on updating to CKEditor5.

@import-keshav import-keshav removed their assignment Mar 23, 2019
@import-keshav import-keshav moved this from In progress to To do in Bug fixing team Mar 23, 2019
@seanlip seanlip changed the title Spaces above images in the RTE should be deletable. [requires upgrade to CKEditor5] Spaces above images in the RTE should be deletable. Mar 29, 2019
@seanlip seanlip changed the title [requires upgrade to CKEditor5] Spaces above images in the RTE should be deletable. [Requires upgrade to CKEditor5] Spaces above images in the RTE should be deletable. Mar 29, 2019
@NishealJ NishealJ self-assigned this Jul 23, 2019
@NishealJ NishealJ moved this from To do to In progress in Bug fixing team Jul 23, 2019
@NishealJ NishealJ moved this from In progress to Serious bugs in Bug fixing team Sep 7, 2019
@NishealJ NishealJ moved this from Serious bugs to To do in Bug fixing team Sep 7, 2019
@NishealJ NishealJ removed their assignment Sep 7, 2019
@kevintab95 kevintab95 added this to Awaiting Triage in Learner and Creator Experience via automation Feb 14, 2020
@iamprayush iamprayush moved this from Awaiting Triage to Medium Priority in Learner and Creator Experience Feb 23, 2020
@nithusha21 nithusha21 removed this from To do in Bug fixing team Mar 4, 2020
@kevintab95 kevintab95 moved this from Medium Priority to Editor Pages in Learner and Creator Experience Aug 16, 2020
@Showtim3 Showtim3 moved this from Editor Pages to RTE, Migrations, and one-off jobs in Learner and Creator Experience Aug 16, 2020
@seanlip seanlip moved this from General learner-facing pages (@donosco98) to Editor Pages (@Showtim3) in Learner and Creator Experience Aug 30, 2020
@nithinrdy nithinrdy removed this from Editor Pages (@kevintab95) in Learner and Creator Experience Oct 8, 2022
@SanjaySajuJacob SanjaySajuJacob added Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. needs debugging Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Projects
Status: Todo
Development

No branches or pull requests

6 participants