Skip to content

Make the image remove option work#3837

Merged
bjester merged 7 commits intolearningequality:hotfixesfrom
rtibbles:assessment_images
Nov 23, 2022
Merged

Make the image remove option work#3837
bjester merged 7 commits intolearningequality:hotfixesfrom
rtibbles:assessment_images

Conversation

@rtibbles
Copy link
Copy Markdown
Member

@rtibbles rtibbles commented Nov 22, 2022

Summary

Description of the change(s) you made

  • Ensures that images get properly cleared when using the 'remove' option from the context menu
  • Create remove event mechanism for clearing custom span elements for custom markdown fields.
  • Consolidates all blank space wrapping in custom markdown field logic.
  • Cleans up squire keydown handling.
  • Modifies prop values using web component interface rather than directly modifying a component prop
  • Tweak markdown conversion to prevent addition of excess line breaks, which would stop images and formulae from appearing inline with each other

Manual verification steps performed

  1. Added an image to an exercise question.
  2. Pressed close.
  3. Reopened the question for editing, used the context menu to remove the image.
  4. Used arrow keys to navigate around the editor window
  5. Added line breaks and removed line breaks between images

Screenshots (if applicable)

Screencast.from.11-22-2022.02.28.17.PM.webm
Screencast.from.11-23-2022.02.31.18.PM.webm

References

Fixes #3817

@rtibbles rtibbles requested a review from bjester November 22, 2022 22:33
Copy link
Copy Markdown
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still noticing some differences with this method vs with the cursor next to the image and inputting backspace. It seems that this still leaves an image wrapper, and that becomes a pesty blank space that will only be removed if you explicitly select it with the cursor and input backspace, or close and reopen the exercise.
Screenshot from 2022-11-22 15-56-55

@rtibbles
Copy link
Copy Markdown
Member Author

Oh, interesting - I guess the existing destroy wasn't doing enough. Will look in more detail!

@rtibbles
Copy link
Copy Markdown
Member Author

So, I've managed to make the span delete itself, but the   on either side that both get deleted with a simple backspace are still not removed.

@rtibbles
Copy link
Copy Markdown
Member Author

OK, this should all be working as intended now. I've updated the PR description with the additional fixes and an updated screencast.

Copy link
Copy Markdown
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@pcenov
Copy link
Copy Markdown
Member

pcenov commented Dec 6, 2022

@rtibbles I was able to identify the following issues:

  1. The images are not displayed in Kolibri once the channel has been imported there:
    2022-12-06_13-48-15
  2. After I remove an image and then I click again in the editor the image reappears. Happens in both Chrome and Firefox:
2022-12-06_15-10-43.mp4

@rtibbles
Copy link
Copy Markdown
Member Author

rtibbles commented Dec 6, 2022

The images are not displayed in Kolibri once the channel has been imported there:

For this, how was this image added? Was it pasted into the text box, added using the 'add image' button?

After I remove an image and then I click again in the editor the image reappears. Happens in both Chrome and Firefox:

Interesting, this must be an issue with the data not being persisted properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants