[4.0][RFC] Serialise inline scripts and inline styles by dgrammatiko · Pull Request #14240 · joomla/joomla-cms

dgrammatiko · 2017-02-24T21:12:39Z

Pull Request for Issue # .

Summary of Changes

Drop the separate arrays for inline scripts and styles
The data will be merged in the relative scrips and styleSheets arrays (can we remove the underscore???)
Make JHTML compatible with these changes

Testing Instructions

Apply patch and observe the produced head, inline scripts should be between script tags with src, same for the styles
Also the jQuery.noConflict(); should be inline and not a file...

Update to JHtml

for inline script:

JHtml::_('script', '', ['inline' => ‘content for inline script’]);

For inline style:

JHtml::_('stylesheet', '', ['inline' => ‘content for inline script’]);

Actual result

Check the structure of the head:

Documentation Changes Required

YUP!

@wilsonge @mbabker @yvesh

yvesh · 2017-02-24T21:44:23Z

libraries/cms/html/jquery.php

 		if ($noConflict)
 		{
-			JHtml::_('script', 'system/jquery-noconflict.min.js', array('version' => 'auto', 'relative' => true));
+			JHtml::_('script', '', [], ['content' => 'jQuery.noConflict();']);


Not better to have that in the array params?

array('inline' => true) ?

Ah okay you changed it again.. what's the difference to doing addScriptDeclaration?

it's not a file, so we need to append the data in a <script> tag.

This is the less intrusive approach (I think for the function)

to ```php JHtml::_('script', '', ['inline' => ‘content for inline script’]); ```

yvesh · 2017-02-24T21:54:58Z

libraries/joomla/document/html.php

-		$this->_scripts     = (isset($data['scripts']) && !empty($data['scripts'])) ? $data['scripts'] : $this->_scripts;
-		$this->_script      = (isset($data['script']) && !empty($data['script'])) ? $data['script'] : $this->_script;
+		$this->styleSheets = (isset($data['styleSheets']) && !empty($data['styleSheets'])) ? $data['styleSheets'] : $this->styleSheets;
+		$this->scripts     = (isset($data['scripts']) && !empty($data['scripts'])) ? $data['scripts'] : $this->scripts;


yvesh · 2017-02-24T21:56:31Z

libraries/joomla/document/document.php

-	 * @since  11.1
-	 */
-	public $_script = array();
+	public $scripts = array();


A lot of templates use $_script and $_style to unset things like jquery migrate. We need to deprecate these.. maybe we even make them protected and make getters / setters for them?

@rdeutz made a PR for removing values there btw.

Yup, that's the reason I did this as an RFC. There is a B/C break here that needs to be justified

We can still use the vars

public $_script = array();

But there gonna be empty, so no clue if that will help. Also there is no $framework anymore due to Mootools removal...

yvesh · 2017-02-24T22:00:09Z

libraries/joomla/document/renderer/html/head.php

 			}

-			$buffer .= '<link href="' . $src . '" rel="stylesheet"';
+			$buffer .= $src !== 'inlineStyles_' . $enumarateInlineStyles ? '<link href="' . $src . '" rel="stylesheet"' : '<style>';


Can we have that in a jlayout? :-) There is sometimes the need to override something

I'd be careful about putting this stuff into layouts. At this point you are rendering the HTML for the <head> element. What are you really going to need to be able to override here that should be encouraged through the use of a layout over the existing onBeforeCompileHead event?

Hmm let me make an example: The way it's rendered now is xhtml (hardcoded) .e.g <link .. />, maybe you want to use html5 (<link ..>) or looking into the future maybe your output is something totally different (xhtml2, html6, xslt, xdoc, pdf ..) which requires a different markup of the inclusion of css and script files.

We don't know what our users are going to do with Joomla, but we should give them the flexibility to override anything.. And try to not hardcode a single line of HTML output in PHP.

Hmm, I think @mbabker is right here, in the sense that this is the HTML head, so let's stick to the platform (w3c) and HTML5. @yvesh for the other examples you mentioned above I guess we need a pdf, xdoc etc render per se, or am I getting this wrong?

Yeah use case is limited, but maybe there will be a new draft for a new cool html version coming in 2 years, which uses again a different markup.. i don't know i still like a bit flexibility here.

If the standard changes, we address it then. Any changes would more likely be with the allowed attributes versus changes to the actual markup, and we already allow attributes arrays so it shouldn't be a big deal.

As for the XDOC, PDF, etc. type outputs, use a renderer for those view formats as needed. This is a renderer specific to the HTML format. It might be a good base to copy down the road, there might be ways to move some stuff to a trait, but let's keep the scope in this class limited to its current design.

wilsonge · 2017-02-25T00:07:23Z

Ok so what's the advantage of this? Like if I was building JDocument from scratch i would probably do something like this. But I don't see the advantage of changing the ordering that inline scripts/styles and files get loaded given how long it's been this way at this point

dgrammatiko · 2017-02-25T06:47:05Z

Two simple examples to point out how the system fails badly with its current architecture:
CSS
com_something, loads:

bootstrap.css: .btn { background-color: white;}
inline css: .btn { background-color: black;}
mod_something loads mod.css with .btn { background-color: red;}

Expected colour red, actual black

JS
com_something, loads:

core.js Joomla.SubmitForm = function1
inline script Joomla.SubmitForm = function2
mod_something loads mod.js with Joomla.SubmitForm = function3

Expected function3, actual function2

Both Css and JS need to be appended in the order they had been inserted... :)

dgrammatiko · 2017-03-13T12:10:29Z

I guess this won't go anywhere, closing

dgrammatiko · 2019-01-12T11:48:05Z

@wilsonge @yvesh @mbabker can we reconsider this one, if I do it in a B/C way?
Reason: I'm trying to utilise type="module" nomodule in my apps but I need some inline polyfill for Safari 10.1 which is impossible to do right now without slipping out of the API...

dgrammatiko · 2019-03-09T21:00:26Z

Closing, obviously Joomla 4 will be out of sync with the current Javascript and as people keep telling me I shouldn't run after unicorns (in this case js modules)...

joomla-cms-bot added the PR-4.0-dev label Feb 24, 2017

opps I missed something

bf787e6

yvesh reviewed Feb 24, 2017

View reviewed changes

dgrammatiko added 2 commits February 24, 2017 23:46

remove jquery-noconflict.js

faeafc4

simplify it

0a2ee63

to ```php JHtml::_('script', '', ['inline' => ‘content for inline script’]); ```

yvesh reviewed Feb 24, 2017

View reviewed changes

dgrammatiko closed this Mar 13, 2017

dgrammatiko deleted the serialise_tags branch July 22, 2017 20:16

dgrammatiko restored the serialise_tags branch January 12, 2019 11:41

dgrammatiko reopened this Jan 12, 2019

joomla-cms-bot added the RFC Request for Comment label Jan 12, 2019

dgrammatiko closed this Mar 9, 2019

dgrammatiko deleted the serialise_tags branch March 9, 2019 21:00

dgrammatiko mentioned this pull request Feb 2, 2020

[4.0] WebAsset support for inline assets #27756

Merged

Uh oh!

Comments

Conversation

dgrammatiko commented Feb 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Testing Instructions

Update to JHtml

Actual result

Documentation Changes Required

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yvesh Feb 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yvesh Feb 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yvesh Feb 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilsonge commented Feb 25, 2017

Uh oh!

dgrammatiko commented Feb 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dgrammatiko commented Mar 13, 2017

Uh oh!

dgrammatiko commented Jan 12, 2019

Uh oh!

dgrammatiko commented Mar 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dgrammatiko commented Feb 24, 2017 •

edited

Loading

yvesh Feb 24, 2017 •

edited

Loading

yvesh Feb 25, 2017 •

edited

Loading

yvesh Feb 25, 2017 •

edited

Loading

dgrammatiko commented Feb 25, 2017 •

edited

Loading