script_tag(): cosmetic for value-less attributes

[xlii-chl]

Description

For <script /> elements, some attributes are usually written without values, for example :
<script defer async src="..." />

This is purely cosmetic as boolean attributes should also be accepted with an empty
value or a value identical to the attribute's name :
https://dev.w3.org/html5/pf-summary/infrastructure.html#boolean-attribute

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

Thank you for sending a PR!

It seems you forgot GPG signing.
https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#signing

[kenjis]

@xlii-chl There are some coding style violations. Please fix them.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#php-style

   1) CodeIgniter4/system/Helpers/html_helper.php (is_null, unary_operator_spaces, not_operator_with_successor_space)
      ---------- begin diff ----------
--- /home/runner/work/CodeIgniter4/CodeIgniter4/system/Helpers/html_helper.php
+++ /home/runner/work/CodeIgniter4/CodeIgniter4/system/Helpers/html_helper.php
@@ -208,7 +208,7 @@
                 }
             } else {
                 // for attributes without values, like async or defer, use NULL.
-                $script .= $k . (is_null($v) ? ' ' : '="' . $v . '" ');
+                $script .= $k . (null === $v ? ' ' : '="' . $v . '" ');
             }
         }
 

      ----------- end diff -----------

   2) CodeIgniter4/tests/system/Helpers/HTMLHelperTest.php (trim_array_spaces)
      ---------- begin diff ----------
--- /home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Helpers/HTMLHelperTest.php
+++ /home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Helpers/HTMLHelperTest.php
@@ -250,7 +250,7 @@
 
     public function testScriptTagWithSrc()
     {
-        $target   = [ 'src' => 'http://site.com/js/mystyles.js' ];
+        $target   = ['src' => 'http://site.com/js/mystyles.js'];
         $expected = '<script src="http://site.com/js/mystyles.js" type="text/javascript"></script>';
         $this->assertSame($expected, script_tag($target));
     }
@@ -257,7 +257,7 @@
 
     public function testScriptTagWithSrcWithoutProtocol()
     {
-        $target   = [ 'src' => 'js/mystyles.js' ];
+        $target   = ['src' => 'js/mystyles.js'];
         $expected = '<script src="http://example.com/js/mystyles.js" type="text/javascript"></script>';
         $this->assertSame($expected, script_tag($target));
     }
@@ -264,7 +264,7 @@
 
     public function testScriptTagWithSrcAndAttributes()
     {
-        $target   = [ 'src' => 'js/mystyles.js', 'charset' => 'UTF-8', 'defer' => '', 'async' => null ];
+        $target   = ['src' => 'js/mystyles.js', 'charset' => 'UTF-8', 'defer' => '', 'async' => null];
         $expected = '<script src="http://example.com/js/mystyles.js" charset="UTF-8" defer="" async type="text/javascript"></script>';
         $this->assertSame($expected, script_tag($target));
     }

      ----------- end diff -----------

https://github.com/codeigniter4/CodeIgniter4/runs/5892697479?check_suite_focus=true

[xlii-chl]

Hello,

thanks for reviewing.

Sorry about the misses, I thought the gpg-signing etc. were for the core members or reviewers. Since signing modify the commit, I'll force-push in a bit if it's okay with you.

I'll put the is_null($v) ? ... → null === $v ? ... change in a separate commit (it seemed kinda less readable to me : needs to recall the operator precedence) so you can omit it if you feel the same way.

[kenjis]

Fore push is okay.

git rebase is recommended way to sync the PR branch.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#pushing-your-branch

I'll put the is_null($v) ? ... → null === $v ? ... change in a separate commit (it seemed kinda less readable to me : needs to recall the operator precedence) so you can omit it if you feel the same way.

The coding style change is a must. If you don't fix it, GitHub Action check fails, and we can't merge this PR.

[kenjis]

@xlii-chl Thank you for updating!

One last thing, we need the User Guide update.
https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#documentation

[xlii-chl]

Hello,

For the doc, I've added a mention in the example:

--- a/user_guide_src/source/helpers/html_helper/011.php
+++ b/user_guide_src/source/helpers/html_helper/011.php
@@ -1,6 +1,6 @@
 <?php
 
-$script = ['src' => 'js/printer.js'];
+$script = ['src' => 'js/printer.js', 'defer' => null];
 
 echo script_tag($script);
-// <script src="http://site.com/js/printer.js" type="text/javascript"></script>
+// <script src="http://site.com/js/printer.js" defer type="text/javascript"></script>

For such a simple thing, it seemed enough to me but do you feel we should add more ? Since I am not a native english speaker, I may need help to write something good.

Or do you mean the Changelog ?
This change may be a little trivial for that, but what about, in the Enhancements section :

script_tag() in the html helpers now able to concisely write boolean attributes : <script src="..." defer />.

[kenjis]

I thought we need explanation for the parameter $src, but as you say the sample code is enough.

Please update:

1. Parameters $src type
   https://codeigniter4.github.io/CodeIgniter4/helpers/html_helper.html#script_tag

mixed is vague and useless.

    :param  array|string  $src: The source name of a JavaScript file

2. Change Log
   https://codeigniter4.github.io/CodeIgniter4/changelogs/v4.2.0.html#enhancements

I think all enhancements should be included the changelog, otherwise users don't know the enhancements.

How about like this?

- HTML helper ``script_tag()`` can now pass ``null`` as the value of the parameter ``$src`` array, allowing boolean attributes to be described concisely. See the sample code for :php:func:`script_tag`.

And could you also update the param type in PHPDoc? It is not mixed but array|string.

CodeIgniter4/system/Helpers/html_helper.php

Line 192 in a8d5b89

* @param mixed $src Script source or an array

Don't forget to run composer cs-fix.

[kenjis]

LGTM