[#22926] - [3.9] Move confirm consent to use the bs modal vs mootools
- Fixed in Code Base
- 13 Nov 2018
- Medium
- Build: staging
- # 22926
- Diff
- zero-24:bs_modal
User tests: Successful: Unsuccessful:
Partial Pull Request for Issue #22908
Summary of Changes
This replaces the Mootools modal vs a bootstrap modal
Testing Instructions
- Install 3.9.0
- enable the
Content - ConfirmConsentPlugin - configure this plugin to point to a article
- open a contact page on the website
- make sure the title is clickable and opens an modal
- apply this patch
- make sure it is still opening a modal this time it should be a bs modal
Expected result
bs modal
Actual result
mootools modal
Documentation Changes Required
none.
Additional Information
When this here gets approved & tested I can apply similar code to the other places where we use the old mootools modal:
- TOS Field:
- Privacy Field:
- Terms Field:
joomla-cms/plugins/user/terms/field/terms.php
Line 101 in 40dcca8
cc @brianteeman as he is the original author of the modal things in this plugin touched here.
Votes
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End Plugins |
Can you please let me know what you want to change? As beyond the (already hardcoded) hight and width I did not add any extensive new HTML part in that plugin right? Or is there something special you want me to change?
There should be a tmpl folder with a layout (default.php or whatever) so people could actually do whatever they want in the front end, in other words, separate concerns. Check the https://github.com/joomla/joomla-cms/tree/staging/plugins/system/stats which was done correctly. This thing right now is hardcoded you cannot override anything unless you rewrite the whole plugin...
| Labels |
Added:
?
|
||
While it is true that using a layout is better, this PR is valid by itself. Moving the thing to a layout can be done easily in a separate PR as well and is actually preferred. Since it makes the PRs more atomic and easier to test and review.
While it is true that using a layout is better
I'll say that it's mandatory, not better. You're not in Joomla 1.5 designers and site builders need to have the ability to override the output without rewriting the plugin! My 2c anyways
It should be a separate pull request for exactly the reasons @Bakual already stated. Nobody is saying not to move the code to layouts but when you start adding all sorts of extras onto pull requests they end up being too complex to properly test and review and people either ignore them or they end up getting merged while being completely broken.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22926.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22926.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22926.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22926.
rdeutz
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-11-13 21:04:55 |
| Closed_By | ⇒ | rdeutz | |
| Labels |
Added:
?
|
||
@zero-24 please use a layout, do not do html markup inside the plugin!