Skip to content

Conversation

habibayman
Copy link
Member

@habibayman habibayman commented Aug 4, 2025

Summary

This PR includes integrating the editor where it should be in studio & removing all traces of the old editor which includes:

  • swapping old/new editors everywhere in place in studio
  • deleting the contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/ folder
  • removing all related dependencies, more info can be found in the issue description

The specs for fitting the new editor also included some changes to the exercise editing UI, mainly to allow for more place to fit the editor.

Note

I added a new functionality that wasn't in the old editor but in my opinion makes sense to be there, I can use your opinion on this.
The functionality is: minimizing the editor on click outside, not just on the click of the minimize icon


References


Reviewer guidance

Try a full editing experience with no constraints this time 😃

  1. You'll notice that the Uploader component in AssessmentItemEditor.vue is now of no use, I have removed it in PR #5271 : a18f114
  2. Also maybe check if there are any traces of the old coded that are still there that need to be deleted.

@nucleogenesis nucleogenesis self-requested a review August 6, 2025 14:56
@marcellamaki marcellamaki changed the base branch from unstable to rich-text-editor August 6, 2025 15:36
@habibayman habibayman force-pushed the feat/integrate-new-RTE branch from 8c702b4 to ffc5736 Compare August 7, 2025 16:31
@habibayman habibayman marked this pull request as ready for review August 7, 2025 16:43
@habibayman habibayman changed the base branch from rich-text-editor to unstable August 9, 2025 21:37
@marcellamaki marcellamaki self-assigned this Aug 13, 2025
@marcellamaki
Copy link
Member

Hi @habibayman - amazing work on this! It's so exciting to see the new text editor in place. I can see that Misha has added some comments for you regarding optimizations and I know you'll be looking into that for your next PR.

I still have some code review to do -- so far my comments are just about some UX things that I think we could improve (and I think they make the most sense to include in this PR) before Radina starts QA, since we've already "found" them.

  1. This wasn't specced anywhere, it's just something I'm noticing now, but I think the toggling the editor experience is a bit odd if there is no question. Perhaps we can see if Jessica has some suggestions -- the two that come to mind for me are 1) if there is no text, don't enable minimizing the toolbar, or 2) adding a placeholder text "you have not added a question yet. click the edit button to add text." or something to this effect (better wording)
Screen.Recording.2025-08-13.at.10.16.40.AM.mov
  1. Relatedly, if the question editor is closed, the keyboard focus outline skips the edit button, and there is no way for the keyboard user (or screen reader user) to re-open the editing state - whether or not there is any content there. [other problems with studio focus outline are not your problem, although if you feel inclined to make sure the 3-dot menu in the top right of the question section is also included in the focus flow, that would be a wonderful bonus, since that seems to be skipped too]
Screen.Recording.2025-08-13.at.10.20.04.AM.mov
  1. This may be included with some of what Misha mentioned, and/or something that would be more suitable to your options PR, (and I suspect this is pre-existing behavior), but the validation on every keypress within the editor seems unnecessary. One of the reasons that this seems unnecessary is because for the things we are checking for the node completeness (like title, license, completion criteria, etc.), most of these are in the "Details" tab, and making edits to the question will have no impact on those.

I think this would be worth raising for conversation in #studio-dev, to think about where we may want to call this validation that is more relevant and efficient.

Screen.Recording.2025-08-13.at.10.23.51.AM.mov
  1. I'm not sure if this is specific to some setting in my browser, but I am getting different paste outputs when I use the text editor buttons vs. the system ctrl + v keyboard shortcut. This could be something that we ask Radina to verify/test across different setups and OSes, and resolve in follow up. once I minimize (and then re-open) the editor, the tag is stripped, so this doesn't seem like a high priority issue in terms of the end outputs, but it may be confusing for users.
Screen.Recording.2025-08-13.at.10.27.48.AM.mov

@habibayman
Copy link
Member Author

habibayman commented Aug 13, 2025

Hi @marcellamaki Thanks!
I know we went over this today in the meeting already but I'll just add what we talked about here for reference, will work as a recap for @nucleogenesis too!

  1. (pre-existing in studio) Yes I think this looks odd too. I noticed you get an error message to indicate that you have an empty question, this only appears after you've pressed the "close" question button. I have an intuition that this was an intentional UX, maybe to not rush showing red annoying errors to the user until they decide to move from the "incomplete" question they're currently editing?
    all in all -- I agree with you that taking Jessica's opinion on this is a good action to take anyways!
image
  1. (pre-existing in studio) Oh - that's a very nice/important catch thank you! I'll be pushing the fix to this PR. I think it's going to be an easy fix too :)

  2. (pre-existing in studio) Yes the validation works like that for questions&hints only, I agree with you on raising this on #studio-dev since this has been there for years now and it may have been intentional for a non-obvious reason.

  3. It's working fine on my end 😅 (I'm on brave browser which is chromium based - if this is a browser issue). I think it's better if we wait till Jacob tries it too, if the issue persists I'll try changing my browser and see the problem.

@habibayman
Copy link
Member Author

habibayman commented Aug 18, 2025

I fixed the main problem which is not being able to open questions/answers for keyboard/screen reader users.
for the questions it was easy, the edit icon was coded as an icon only, so I wrapped it in a button.
as for answers/hints, I made the editor itself focusable and mapped clicking "enter" to emitting an event that open the answer/edit because there were no place for a new button. and here are demos of how they work now.

tab-to-question.webm
tab-to-answer.webm

There're still few weird behaviors of how the focus flow goes in the exercise editor in general, I used a browser extension to test that 😅 I just don't think they fit in this PR since I will not be editing anything related to the editor. But yes I would like to work on this I don't mind that. I'd like to coordinate that with Radina first before opening a separate issue :)

@marcellamaki --

the 3-dot menu in the top right of the question section is also included in the focus flow

when I tried it, it did get included, just after you've gone over all the answers/hints in the section, and that's what I meant with "weird behavior" that we need to decide on :)

@habibayman habibayman force-pushed the feat/integrate-new-RTE branch from eb07cfe to e46709e Compare August 22, 2025 17:45
Copy link
Member

@marcellamaki marcellamaki left a comment

Choose a reason for hiding this comment

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

Blocking feedback has been addressed. Between the other open PRs for optimizations, plus looking forward to Radina's QA for follow up tasks and further refinement, this is ready to get merged 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

Great work, @habibayman 👏

@marcellamaki marcellamaki merged commit 58bca1b into learningequality:unstable Aug 22, 2025
13 checks passed
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.

[New Rich Text Editor]: Completely replace the old editor
2 participants