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

[4.0][bugfix] Fix Figure misbehaviour in tinyMCE #36221

Merged
merged 2 commits into from
Dec 21, 2021

Conversation

dgrammatiko
Copy link
Contributor

Pull Request for Issue, sorry couldn't found the one...

Summary of Changes

  • Adds a class image to the figure element as tinyMCE expects that to enable the editing of the figcaption text

Testing Instructions

This Pr needs NPM

  • Fetch and run npm install
  • Try to select an image using the Joomla dropdown icon and then selecting media
  • Add some figure class and some text for the figure caption
  • You should have a boxed area around the image and clicking in the caption text you should be able to edit it

Screenshot 2021-12-05 at 18 56 31

Screenshot 2021-12-05 at 18 56 48

Actual result BEFORE applying this Pull Request

Editing a figure element is kinda broken

Expected result AFTER applying this Pull Request

Editing a figure element is easier

Documentation Changes Required

No

@brianteeman @N6REJ

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Dec 5, 2021
@dgrammatiko dgrammatiko changed the title Fix Figure misbehaviour in tinyMCE [4.0] Fix Figure misbehaviour in tinyMCE Dec 5, 2021
@dgrammatiko dgrammatiko changed the title [4.0] Fix Figure misbehaviour in tinyMCE [4.0][bugfix] Fix Figure misbehaviour in tinyMCE Dec 5, 2021
@N6REJ
Copy link
Contributor

N6REJ commented Dec 5, 2021

I have tested this item ✅ successfully on c54a3fe

Works exactly as expected


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36221.</sub
Screenshot 2021-12-05 122245

@N6REJ
Copy link
Contributor

N6REJ commented Dec 5, 2021

@dgrammatiko nice and simple. If I could make a user request might be handy to have a.. switch(?)... to move caption before/after image.

@dgrammatiko
Copy link
Contributor Author

if I could make a user request might be handy to have a.. switch(?)... to move caption before/after image.

Do you mean something like:

<!-- before -->
<figure>
  <figcaption>
  <img ... >
</figure>

<!-- after -->
<figure>
  <img ... >
  <figcaption>
</figure>

TBH I had to check MDN as I had the impression that figcaption could only go as the last element of the figcaption, and clearly, that's not what the specs say: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/figcaption. Give me some time to think how that could be done

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on c54a3fe


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36221.

@brianteeman
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on c54a3fe


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36221.

@dgrammatiko
Copy link
Contributor Author

@brianteeman what failed here?

@brianteeman
Copy link
Contributor

oops sorry I meant to change it to untested

@brianteeman
Copy link
Contributor

I have not tested this item.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36221.

@brianteeman
Copy link
Contributor

tested correctly now


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36221.

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on c54a3fe


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36221.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36221.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Dec 6, 2021
@N6REJ
Copy link
Contributor

N6REJ commented Dec 7, 2021

if I could make a user request might be handy to have a.. switch(?)... to move caption before/after image.

Do you mean something like:

<!-- before -->
<figure>
  <figcaption>
  <img ... >
</figure>

<!-- after -->
<figure>
  <img ... >
  <figcaption>
</figure>

TBH I had to check MDN as I had the impression that figcaption could only go as the last element of the figcaption, and clearly, that's not what the specs say: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/figcaption. Give me some time to think how that could be done

ty, thats exactly what I meant. I appreciate you doing the extra effort

@Quy Quy added this to the Joomla 4.0.6 milestone Dec 21, 2021
@Quy Quy merged commit 5f8fb0d into joomla:4.0-dev Dec 21, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 21, 2021
@Quy
Copy link
Contributor

Quy commented Dec 21, 2021

Thanks

@dgrammatiko dgrammatiko deleted the patch-11 branch December 21, 2021 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants