Skip to content

[wip] Modal Field retake#39351

Closed
dgrammatiko wants to merge 3 commits intojoomla:4.3-devfrom
dgrammatiko:4.3-dev-modals-xss
Closed

[wip] Modal Field retake#39351
dgrammatiko wants to merge 3 commits intojoomla:4.3-devfrom
dgrammatiko:4.3-dev-modals-xss

Conversation

@dgrammatiko
Copy link
Copy Markdown
Contributor

Pull Request for Issue # .

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.3-dev labels Dec 3, 2022
@Septdir
Copy link
Copy Markdown
Contributor

Septdir commented Dec 7, 2022

Can I use this field in subform? If not, then that's a big problem.

@dgrammatiko
Copy link
Copy Markdown
Contributor Author

dgrammatiko commented Dec 7, 2022

Can I use this field in subform? If not, then that's a big problem.

yes, the idea of using a custom element as the wrapper was also because of the sub forms, this way you need 0 js to handle adding removing a field. But don’t take my word, try it yourself with either user or media fields

EDIT: Sorry @Septdir I thought this was another PR so the user/media fields are totally irrelevant here. Theoretically, this implementation should work ootb with subforms due to the fact that the field is now a Custom Element. I say theoretically because:

  • the original code is not mine, I just revived the code and applied some changes so it works on 4.3
  • I haven't tested that case

@Septdir
Copy link
Copy Markdown
Contributor

Septdir commented Jan 9, 2023

Sorry @dgrammatiko, unfortunately, I have my own implementation of modal fields, it is not suitable for Joomla core, because it is much more complicated and uses ajax, which allows you to add additional data to the hidden field for use in onchange and onload events.

And unfortunately, I won't be able to test this implementation of the field right now, I just don't have enough time for everything. I can only say that the most important thing is that media and modal_article work in subform.

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.

3 participants