Missed few mootools modals on com_menus boostrapization

[dgrammatiko]

This is complimentary to #4661

• Some modals for selecting a single article, contact and newsfeed are still based on mootools code
• Also a disabled="disabled" was added to the menu type input and that prohibits validation to act properly, thus is now removed
• The validation for the modal field wasn’t very descriptive
• Editing a module in Module Assignment and then canceling the changes will result in a lock icon appearing in module manager
• All modals have a footer with close button and modules modal also have a save & close button as well.

testing

1. Create a menu, add some title and alias and press save you should see:
   
2. On the home page menu open module assignment click on a module to bring the modal and the click cancel. Go to module manager and verify that no lock icon exist in front of the module you edit in homepage menu.
3. Create a new menu and select as Menu Item Type singe article then click on select button on the next field Select Article respectively you should see a bootstrap modal:
   
   Repeat this also for single contact single newsfeed with a selection on Select Contact Select Newsfeed respectively.
4. Also Repeat the step 2 insert a title but DON’T select an article, newsfeed and contact and press save you should see a descriptive message about the field that needs input (previously you would only see an error as a message!) Thanks @Erftralle for coding this!

Preview

[n9iels]

@test works fine for me, thanks!

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6804.}

[Erftralle]

@test: Thanks a lot. I confirm that the client side validation of the menu item type is working fine now.
The disappearance of the article Select button (in the case a menu item type Single article has been selected) after closing the modal to edit some module options (tab Module Assignment) has also been fixed with this PR.

During the test I noticed

• that the problem with the disappearance of the Select button will still occur if selecting the menu item types Single contact and Single newsfeed (still mootools modals).
• that the modal to edit some module options (tab Module Assignment) shows two vertical scrollbars
• the client side validation after selecting a menu item of type Single article when not selecting any and trying to save it only throws the message Error where I would expect also the additional information Invalid field: Select article

I don' t know exactly , if the modal in administrator/components/com_categories/models/fields/modal/category.php should also be mentioned here because it seems not to be selectable as menu item type.

[dgrammatiko]

@Erftralle Thanks I posted some changes:
single contact and single newsfeed now render a bootstrap modal
double scrollbar eliminated
Client side validation still not very informative, but that is not a problem introduced with #4661
Also category modal is not supposed to be provided as a direct menu. Categories are provided through components e.g. content, contacts, newsfeed etc...

[dgrammatiko]

@roland-d One consideration about #4661 that arose from @Erftralle findings:
If a user has installed a component that uses modal for selecting a single item (e.g. K2 and single item)
Then the link will be for a mootools modal with a class modal. In case the user try to edit a module in module assignment tab, close that module module and come back to Details tab the button for selecting-changing the item will vanish. (this is due to the fact that the module modals close with jQuery(".modal").modal("hide"); which obviously will hide the button that has a .modal class. There are few things that could (or not) be done here:

1. Accept this glitch, document it
2. Change the bootstrap modal to have another class e.g. bsModal (highly unrecommended)
3. Change the code in module assignment so that the modal id is passed and the code for closing the modal acts only on the id of the modal.

I guess option 3 is the only way, but it will make the code a little bit more complicated that’s why I wrote this note before I head to make any changes.
In case these are not accepted options I guess we need to revert #4661 and find another way to move forward to eliminate the mootools modals

[Erftralle]

@test
The double scrollbar disappeared and the Select buttons stay visible now. Thanks a lot.

@DGT41
I have send you dgt41/joomla-cms#12 against your branch regarding the improvement of the client side validation: What do you think of it?

[roland-d]

@DGT41

If a user has installed a component that uses modal for selecting a single item (e.g. K2 and single |item)
Then the link will be for a mootools modal with a class modal.
Are we talking about a link generated by K2 or by Joomla?

Your third option has my preference as accepting the glitch is not even an option since it can be fixed. Using another class might be more work than it's worth.

Could you give me perhaps a clearer example of the problem. I am not entirely sure where the issue is. Thanks.

[dgrammatiko]

@roland-d Try to follow the directions in this comment #4661 (comment)
In 3.4.1 without this patch.

If you try to add a new menu item of type Single article for instance, then change to the tab Module Assignment to open a module for editing, close it again and change to the tab Details back again you will miss the Select button for the field Select article.
I suspect it's because of the introduction of window.parent.jQuery('.modal').modal('hide'); at some places of the code.

Reposted the comment as github seems to get confused…
The conflict exists in a modal button e.g. in case of single article. I ll try to push some code for the 3rd option

[dgrammatiko]

@roland-d 25e6df3 this should make things safe for coexistence of mootools and bootstrap modal on com_menus!

[dgrammatiko]

UPDATES:
All the modal are now bootstrapped!
All the modals have a footer with close button and for the modules also a save & close button!
The validation is simplified thanks to @Erftralle code!

How to test:

Check all the combos of available menu types and the sequential modal that work correctly. Also check that required fields when not filled (selected) throw an understandable error (that shows which field is failing).

The end result should be:

[n9iels]

Thanks! Much better! I also like that footer :)

@test the error for empty required field looks good to me.
I found one 2 very little bugs for the modals:

Bug when opening modal for editing a module

1. Go to Menus -> Menu Manager
2. Click on a Module dropdown with two or more modules
3. Click on the first module, and close the modal with the close button
4. Click on the second module, and notice the modal won't open correctly. Click for the second time, and the modal open correctly

Animation missing
When you close a modal in a menu item, for example the modal for selecting a article, the close animation is missing.

I'm using the latest version of Google Chrome

[dgrammatiko]

@n9iels thanks!
I can’t replicate the problem with the menus - modules here, see the video

About the missing animation, you are right! That’s because when you select a menuitem e.g. single contact the procedure is to reload the form (page), but maybe we can do something there as well, I will try it out later on today...

[n9iels]

@DGT41 oops, I see the mistake. I didn't wait until the page reload after click on the close button. I was a little bit too fast 😄

[dgrammatiko]

@n9iels You were right, that missing animation actually was a forgotten mootools modal close function! It’s fine now.

[n9iels]

@test works fine for me now, thanks!

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6804.}

[roland-d]

When I apply this PR and test it, I get this error in the console:

TypeError: window.parent is null
http://localhost/joomla-cms/administrator/index.php?option=com_modules&view=module&tmpl=component&layout=modal&id=82
Line 30

Why would you want to remove the _id for the ID field?

I see that this "_id" cannot die so easily 😕

I think it is pretty nice, so you know that ID belongs to that name field. As for it being a B/C issue, I do think it is, anyone using the select article in their own extension will be missing the id field as it is now.

[dgrammatiko]

@roland-d Thanks I just corrected that console error.
About that _id I guess is better to revert the validation code here and try to get it right with @Erftralle in another PR.

[Erftralle]

@DGT41: It is a pity that you reverted the validation code.

@roland-d:

Why would you want to remove the _id for the ID field?

The validation does not work properly for these modal form fields. In case of a validation error you just see the word Error in the system message container (without any hint to the regarding field) and the corresponding label is not coloured red. The red colour is also not removed after a selection has done. See my posting above and also #6654 and #6761 for more details.

As for it being a B/C issue, I do think it is, anyone using the select article in their own extension will be missing the id field as it is now.

Could you explain more detailed why do you think that could be a problem.

[Fedik]

I would move the validation issue to the separate pull ...
let's split 🐮 and 🐔

[dgrammatiko]

@Erftralle I am not against correcting the validation but already this PR needs too much testing. Adding here the code for validation as well makes things even harder to get some tests. Lets get this one merged (this PR still needs 2 successful tests) and then open another PR for validation with the code already in hand or an improved version. I will definitely support it, test it etc… Small chunks of code are always easier to get tested and merged than a complicated PR that solve too many bugs

[dgrammatiko]

Hathor also:

[roland-d]

Agree that it is best to move the validation issue to another PR as each PR ideally should deal with a single problem. This makes isolated testing possible and easier.

Could you explain more detailed why do you think that could be a problem.

I have taken a closer look and the original problem I foresaw is not an issue I think. However I still think it will be good to keep the _id field as it matches the _name field. Having the _id should not be an issue when fixing the validation issue I think.

@test All good, both Hathor and Isis the select button remains visible after applying the patch.

[Erftralle]

See #6885 for an alternative suggestion to improve client side validation.