[#32062] - [4.0] Introduce a new render
- Closed
- 2 Feb 2021
- Medium
- Build: 4.0-dev
- # 32062
- Diff
- dgrammatiko:4.0-dev___Toolbar_Modals
User tests: Successful: Unsuccessful:
Pull Request for pending todos
Summary of Changes
-
Create a new renderer bodybottom which renders just before the closing body tag
-
Add a new private variable to the Document
-
Add setter/getter for the previous private variable
-
Fix the @todos for the toolbar modals
Testing Instructions
Apply patch, and check that the modals in the Article Edit form (both the Versions and Preview) are still functional
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
Documentation Changes Required
@wilsonge although the problem was pretty obvious in the BS5 transition this solution and the shenanigans (the ability to enqueue html for the bottom of the page) should be pretty valid for J4 regardless of the BS5 decision
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Layout Libraries |
| Labels |
Added:
?
|
||
If you can think of a better word than queue it will be better in the long term as its so easy to make a typo with it
I'm not good at naming things but I'm open to anything here
This produces invalid HTML.
This produces invalid HTML.
Interested to see how you came up with this conclusion
Fixed
It is a good idea but it will be better if the assets are managed by the assets manager.
We can provide an option position with values bodystart and bodyend. We should rework the script render to parser jdoc blocks depending on the attribute position.
Factory::getDocument()->getWebAssetManager()
->addInlineScript(
HTMLHelper::_('bootstrap.renderModal',
'modal_' . $selector,
[
'url' => $displayData['doTask'],
'title' => $text,
'height' => '100%',
'width' => '100%',
'modalWidth' => 80,
'bodyHeight' => 60,
'closeButton' => true,
'footer' => '<button class="btn btn-secondary" data-bs-dismiss="modal" type="button"'
. ' onclick="window.parent.Joomla.Modal.getCurrent().close();">'
. Text::_('COM_BANNERS_CANCEL') . '</button>'
. '<button class="btn btn-success" type="button"'
. ' onclick="Joomla.iframeButtonClick({iframeSelector: \'#modal_downloadModal\', buttonSelector: \'#exportBtn\'})">'
. Text::_('COM_BANNERS_TRACKS_EXPORT') . '</button>',
]
),
['position' => 'bodyend']
);<head>
<jdoc:include type="scripts" />
</head>
<body>
<jdoc:include type="scripts" position="bodystart" />
.....
....
<jdoc:include type="scripts" position="bodyend" />
</body>
It is a good idea but it will be better if the assets are managed by the assets manager.
This PR is not affecting the script position, it's a way to delegate a chunk of HTML to the end of the document.
Could the scripts refactored to take advantage of the new position?
Definitely but that's for another PR. This one solves a problem that all the Toolbar buttons that trigger a Modal have issues with the z-index (conflicting dropdown and modal) so the rendered Modal HTML needs to be delegated to another place in the document (and since they are Modals could happily exists as the last elements in the page)
Hope this clears things up
PS the HTMLHelper::_('bootstrap.renderModal', []) returns an HTML string not a script
What still needs to be done to merge this PR?
What still needs to be done to merge this PR?
It needs 2 successful tests as every other Pull Request
I see 3 successful checks for this PR, thats why I asking what still needs to be done to merge this PR?
I see 3 successful checks for this PR, thats why I asking what still needs to be done to merge this PR?
Thumbs up is not the way a test result is counted. You need to use https://issues.joomla.org/tracker/joomla-cms/32062 to mark your test as successful or failed
I guess this is not going anywhere
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-02-02 23:09:01 |
| Closed_By | ⇒ | dgrammatiko | |
| Labels |
Added:
Conflicting Files
|
||
Why you closed this pull?
Two reasons:
- no tests after 2+ weeks
- this has an odd edge case
for test you should leave open this
for edge case I don't know
If you can think of a better word than queue it will be better in the long term as its so easy to make a typo with it