[#20788] - [4.2] Fix modal fields
- Closed
- 20 Mar 2022
- Medium
- Build: 4.0-dev
- # 20788
- Diff
- okonomiyaki3000:fix-modal-fields
- Error Hound 1 violation found. Details
- Failure continuous-integration/drone/pr the build failed Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Pending JTracker/HumanTestResults Human Test Results: 0 Successful 1 Failed. Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #11805 .
Summary of Changes
New (abstract) field class ModalField to serve as base for most modal field classes.
New web component joomla-field-modal
Updated modal_category, modal_contact, modal_article, modal_menu, modal_newsfeed to use ModalField as base class.
Testing Instructions
These field types are most easily testable on menu item pages. For example, the 'Single Article' menu item type has a modal_article field.
Since the problem with these field types before was that they did not work properly in subforms, test them in subforms. The fields/checkboxes plugin has a subform in it, you can easily alter that to include fields of these types.
Expected result
Modal behavior should be basically the same as in J3. You can open modals to select, edit, or create items. You can clear the field of all values.
When using modal fields in a repeatable subform, fields on newly created rows should work just as well as those that were on the page when initially loaded.
Actual result
Before the patch, modal fields in repeatable subforms had trouble (#11805 is about modal_article but really the problem applies to all of them). It's also possible that all modal fields were broken in J4 anyway.
After the patch, all of these things should work.
Documentation Changes Required
Maybe not.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_categories com_contact com_content com_menus com_newsfeeds Layout Libraries JavaScript |
@dgrammatiko Ah, thanks! I had a feeling something was wrong.
I guess I also need to add it to package.json in the appropriate places?
| Labels |
Added:
?
|
||
| Category | Administration com_categories com_contact com_content com_menus com_newsfeeds Layout Libraries JavaScript | ⇒ | Administration com_categories com_contact com_content com_menus com_newsfeeds JavaScript Repository Layout Libraries |
This dog is provoking me.
I'm a big fan of linters actually but I strongly disagree with some of the choices in how eslint has been set up. Also, the generated files should not be linted at all.
| Title |
|
||||||
| Title |
|
||||||
This one escaped me - can we sync this back up please?
@okonomiyaki3000
Can you solve conflicts and be nice to hound ?
@infograf768 I'm going to be a bit too busy to get back to it for probably at least a month maybe more. Also I'm not enthusiastic about new js coding standards. Makes me not really want to write js for joomla anymore.
@dgrammatiko I am a newbie in the J4.0 space. Lets see how things go. I worked on 1.5-3.5 versions. Never been into J! codebase for a long time. I hope I can work things out.
@infograf768 thanks for linking the issues.
Is this ready to test yet?
@uglyeoin Nope. Well, if I remember correctly, it should work as it's supposed to. The only problem is that Hound complains about...something. Coding style violations maybe? It's been a while so I don't exactly remember but it's too tedious because I wasn't able to test these violations locally, I just had to upload here and wait for the result then go fix it and repeat. Also, the coding standard Joomla has decided to go with for JS seems pretty awful. I don't remember all the points about it that bothered me but, for example, it forbids the use of bitwise operators. WTF kind of rule is that? Get out of here with that nonsense.
Is it possible to test it now? :-)
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20788.
@coolcat-creations I have not touched it in a long time and I will not until I can run these hound tests locally which I have also not tried to do for a long time. If someone else wants to pick it up, I would not be sad.
Hi before creating a new issues, just asking if this is related.
When in Menu > single article > create new article > modal comes up with new article form.
The published and featured are malformed / not working.
Same for editing a plugin in a modal, all btn btn-group radio buttons do not work any more (they switch value, but the new value is only displayed after refresh page.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20788.
@okonomiyaki3000 Do you have any interest in fixing this or should the PR be closed?
A issue that is not fixed should not be closed.
@roland-d See #20788 (comment)
@Ruud68 If it is no longer being maintained, then close the PR and reopen the issue.
I have tested this item
The file marked for modification does not exist: administrator/components/com_categories/Field/Modal/CategoryField.php
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20788.
@okonomiyaki3000 do you mind if I clone this PR to rebase it?
@dgrammatiko See his last reply #20788 (comment)
richard67
- | Title |
|
||||||
| Category | Administration com_categories com_contact com_content com_menus com_newsfeeds Layout Libraries JavaScript Repository | ⇒ | Administration com_categories com_contact com_content com_menus com_newsfeeds JavaScript Repository Layout Libraries NPM Change |
@dgrammatiko do you mind to make a new pr?
Last time we were discussing about this one (basically with @Fedik #32473) we conclude that there some known bottlenecks and I don't think I have a solution for all aspects of this PR.
For example (quoting @Fedik here but I also agree)
The interaction between the iframe content and the parent document need to be event based,
without hardcoded callbacks, that often missed somewhere. Basically enough just 2 event:
` joomla:close-popup`, `joomla:select-content` (with data to select).
This is platform wide change with some hard B/C breaks, so I don't think this could happen in a minor version, maybe 5.0 (?) but if there's a decision to move towards a better solution in the next minor, I could spend some time to do a POC
BTW dialog is supported everywhere, so there's no need for bootstrap for the modals anymore. Just saying...
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-03-20 12:18:41 |
| Closed_By | ⇒ | laoneo | |
| Labels |
Added:
Ready to take over
NPM Resource Changed
?
Removed: ? |
||
Great work @okonomiyaki3000 !!!
One note here: this file :
media/system/webcomponents/js/joomla-field-modal.es6.jswhich is the source of the custom element needs to be moved here: https://github.com/joomla/joomla-cms/tree/4.0-dev/build/webcomponents/js and then you need to run the commandnode build.js --compilecejsThe reason for this is that the
classon custom elements is not properly transpiled from babel. At some point we need to merge the es6 and the custom elements building scripts...