Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend global attributes array #175

Merged
merged 4 commits into from
Jul 27, 2022

Conversation

Koopzington
Copy link
Contributor

Q A
Documentation no
Bugfix no
BC Break no
New Feature yes
RFC no
QA no

Description

With further development of the HTML standard there have been introduced a number of new global attributes of which some are also applicable on form elements. To support these new attributes i have decided to extend the existing whitelist of global attributes that's being used in the rendering process and added the attributes listed on MDN which were missing.
As suggested by @froschdesign on slack i've commented out the ones i perceived as not really useful in context of form elements.

The following attributes have been added:

  • autocapitalize - automatically cases typed content
  • enterkeyhint - relevant for virtual keyboard contexts
  • inputmode - relevant for virtual keyboard contexts
  • is - support for HTML custom elements
  • itemid - commented out since Microdata on input elements didn't exactly make sense to me
  • nonce - commented out since afaik they're only relevant for <script> and <style> or similar tags
  • part - for CSS styling
  • slot - for shadow DOM usage
  • translate - commented out since the description points to Text nodes inside elements

Extending the list of allowed global attributes

Signed-off-by: Koopz <[email protected]>
@@ -93,18 +93,25 @@ abstract class AbstractHelper extends BaseAbstractHelper
*/
protected $validGlobalAttributes = [
'accesskey' => true,
'autocapitalize' => true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we at least document where these attributes come from? A link to their spec would be helpful 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean inside the code, per attribute?
'autocapitalize' => true, // https://html.spec.whatwg.org/multipage/interaction.html#attr-autocapitalize

I don't know how prone the spec page is regarding breaking links, should they ever decide to move that blurp to another page.

Or a mention on https://docs.laminas.dev/laminas-form/v3/helper/abstract-helper/?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean inside the code, per attribute?
'autocapitalize' => true, // https://html.spec.whatwg.org/multipage/interaction.html#attr-autocapitalize

IMHO yes, per attribute pointing to whatwg.org is ok and enough. I wouldn't care about link stability, we can't be perfect.

Or a mention on https://docs.laminas.dev/laminas-form/v3/helper/abstract-helper/?

Listing each attribute would be cumbersome to me, but a mention that only some are supported with a call to look at Laminas\Form\View\Helper\AbstractHelper::$validGlobalAttributes to have the full list, that would be very welcome

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for taking the time to expand on the references.

As for the CS failures, I think those are on our sude to adjust/fix (probably suppress)

@Koopzington
Copy link
Contributor Author

Looks like i was a bit too hasty trying to fix things myself.
Should we use the snapshotted links now or not? :)

@Ocramius
Copy link
Member

Ocramius commented Jul 7, 2022

I don't mind - these are usually very stable regardless 👍

Copy link
Contributor

@Slamdunk Slamdunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I'll try to fix the CI once back from vacation 🏖️

@Ocramius Ocramius modified the milestones: 3.3.0, 3.4.0 Jul 25, 2022
@Ocramius Ocramius changed the base branch from 3.3.x to 3.4.x July 25, 2022 17:24
@Slamdunk Slamdunk merged commit c94df30 into laminas:3.4.x Jul 27, 2022
@Slamdunk
Copy link
Contributor

Thank you @Koopzington

@Koopzington Koopzington deleted the more-global-attributes branch July 29, 2022 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants