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

Embed question implementation for TinyMCE #1

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

vuvanhieu143
Copy link

No description provided.

@timhunt timhunt merged commit e830c56 into moodleou:main Mar 15, 2024
4 checks passed
@timhunt
Copy link
Member

timhunt commented Mar 15, 2024

@vuvanhieu143 thanks for working on this, I think we are getting close. Some comments

  1. I think that for a relatively uncommon action like embedding a question, we should just have the entry in the Insert menu, but not a toolbar button.
  2. Therefore, to get the grammar right, the language string should be Insert -> Embedded question (not Embed).
  3. Can you use core/loadingicon to do the loading progress icon? Oh! you already were, it is just that the unused loading string was left behind.

Acutally, because this is new to me, I decided to try to fix these things myself, as a way to engage with the code more, and so understand it better. I hope that is OK.

I know the answer to 1. depends on further internal discussion, but still, I got carried away and pushed it to the main branch anyway.

Since I am new to this sort of code, please could you review the changes I made, and point out any mistakes. Thanks.

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.

2 participants