Skip to content

Conversation

@bart-pywalker
Copy link

@bart-pywalker bart-pywalker commented Nov 29, 2022

This PR adds the playsinline attribute that is used in the html Video component. Per mdn web docs, playsinline is:
"A Boolean attribute indicating that the video is to be played "inline", that is within the element's playback area. Note that the absence of this attribute does not imply that the video will always be played in fullscreen."

Contributor Checklist

  • I have broken down my PR scope into the following TODO tasks
    • test to see if playsinline applied to html video plays video inline instead of full screen with pressing the "play" button (rotated triangle) when iPhone is on low power mode
  • I have run the tests locally and they passed. (refer to testing section in contributing)
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

  • I have added entry in the CHANGELOG.md
  • If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follows
    • this GitHub #PR number updates the dash docs
    • here is the show and tell thread in Plotly Dash community

added playsinline attribute into supportedAttributes array
dash-html-components/scripts/data/attributes.json
added playsinline into BOOLEAN_PROPERTIES array in dash-html-components/scripts/generate-components.js
@alexcjohnson
Copy link
Collaborator

@bart-pywalker thanks for the PR! Looks to me as though it needs to be playsInline rather than playsinline though, does this work as is?

@bart-pywalker
Copy link
Author

@bart-pywalker thanks for the PR! Looks to me as though it needs to be playsInline rather than playsinline though, does this work as is?

couldn't test it. When i tried to set up a developer testing environment, i got this error after running "pip install -e .[ci,dev,testing,celery,diskcache]"

I tried installing Rust / Cargo via the link specified and received the same error again

Preparing metadata (pyproject.toml) did not run successfully.
│ exit code: 1
╰─> [6 lines of output]

Cargo, the Rust package manager, is not installed or is not on PATH.
This package requires Rust and Cargo to compile extensions. Install it through
the system's package manager or via https://rustup.rs/

Checking for Rust toolchain....
[end of output]

Not sure about the camel casing ...
per Mozilla https://developer.mozilla.org/en-US/docs/Web/HTML/Element/video , it is all lower case

@alexcjohnson
Copy link
Collaborator

Ah yes - if it's trying to invoke Rust that means some dependency is trying to compile, rather than installing from a wheel. You probably only need [dev] to get it to build and test manually, but not a big deal, we can take over the PR if you like.

Re camelCase: see https://reactjs.org/docs/dom-elements.html and specifically https://stackoverflow.com/questions/68219114/ios-video-playsinline-not-working-in-reactjs

@bart-pywalker
Copy link
Author

Ah yes - if it's trying to invoke Rust that means some dependency is trying to compile, rather than installing from a wheel. You probably only need [dev] to get it to build and test manually, but not a big deal, we can take over the PR if you like.

Re camelCase: see https://reactjs.org/docs/dom-elements.html and specifically https://stackoverflow.com/questions/68219114/ios-video-playsinline-not-working-in-reactjs

Ah, I see. Sure, that is fine. I made this request before finding out that dash-player got revived and turned into an actual PIP package and am pretty happy with dash-player, but if this gets added, I could eliminate one library from the build

React, not HTML. Gotcha. yea, will need to be 'playsInline'

"video"
],
"description": "Sets the video to be played inline / within the element's playback area."
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is actually auto-generated by scripts/extract-attributes.js by scraping https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes - and unfortunately playsinline is missing there, so when I rerun the build process your edits disappear. We could make a special case for it, but better would be to fix it upstream -> https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes#see_also ("Found a problem with this page?")

Copy link
Author

Choose a reason for hiding this comment

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

Okay, i made an issue request on their github. Now it will just be a waiting game?

Again, I am happy with dash-player, so there is no hurry on my end. Just trying to better understand the build for dash components

this is an generated file ... restoring it to it's original state
@gvwilson gvwilson self-assigned this Jun 14, 2024
@gvwilson gvwilson removed their assignment Aug 2, 2024
@gvwilson gvwilson added feature something new P2 considered for next cycle community community contribution labels Aug 13, 2024
@robertclaus robertclaus requested a review from AnnMarieW December 3, 2025 17:05
@AnnMarieW
Copy link
Collaborator

Hi @bart-pywalker,

I was going through old PRs and this one works now! At some point, playsinline was added to https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes

I'll open a new PR #3534 and reference this one (I don't think I can make changes to yours since I'm not a maintainer). There is certainly no expectation for you to finish this up after such a long delay, but I noticed this was your first PR. 🏆 I'll close mine if you decide you want to see yours merged. It just needs a changelog entry and run the script to extract the attributes. (if it doesn't do that automatically when you run the build, change to dash/components/dash-html-components and run npm run extract

Thanks for your contribution to Dash!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community community contribution feature something new P2 considered for next cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants