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

fix(types): add controlslist to html declarations #6026

Merged

Conversation

rwaskiewicz
Copy link
Member

What is the current behavior?

prior to this commit, using the controlslist attribute in a video or audio element would result in a type checking error. with this commit, we allow the attribute to take one of three values:

  • nodownload
  • nofullscreen
  • noremoteplayback

What is the new behavior?

note that at the time of this writing, only chromium browsers support this attribute. it is the responsibilty of end users to determine if using this attribute is appropraite for their projects.

Documentation

Does this introduce a breaking change?

  • Yes
  • No

Testing

Adding a video or audio element to a stencil component like so no longer creates type errors:

<>
  <video controlslist="noremoteplayback"></video>
  <video controlslist="nofullscreen"></video>
  <video controlslist="noremoteplayback"></video>
</>

Other information

fixes: #6015

prior to this commit, using the `controlslist` attribute in a `video`
or `audio` element would result in a type checking error. with this
commit, we allow the attribute to take one of three values:
- nodownload
- nofullscreen
- noremoteplayback

note that at the time of this writing, only chromium browsers support
this attribute. it is the responsibilty of end users to determine if
using this attribute is appropraite for their projects.

fixes: ionic-team#6015
Copy link
Member

@tanner-reits tanner-reits left a comment

Choose a reason for hiding this comment

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

Welcome back 😂

@rwaskiewicz
Copy link
Member Author

Just passing through the old neighborhood 😉

@rwaskiewicz
Copy link
Member Author

BTW, I no longer have merge permissions, so I'll leave this y'all to merge when it's time

@christian-bromann christian-bromann added this pull request to the merge queue Oct 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 11, 2024
@tanner-reits tanner-reits added this pull request to the merge queue Oct 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 11, 2024
@tanner-reits tanner-reits added this pull request to the merge queue Oct 11, 2024
@tanner-reits tanner-reits removed this pull request from the merge queue due to a manual request Oct 11, 2024
@tanner-reits tanner-reits added this pull request to the merge queue Oct 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 11, 2024
@christian-bromann christian-bromann merged commit f4b48e9 into ionic-team:main Oct 11, 2024
88 checks passed
@christian-bromann
Copy link
Member

This was released with @stencil/[email protected]

@rwaskiewicz rwaskiewicz deleted the fix/add-controlslist-attr branch October 25, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing 'controlsList' Attribute in HTMLVideoElement Type Definition Throws TypeScript Error
3 participants