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

Add playsinline attribute / prop to be used in html.Video component #2338

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

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
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.

None yet

2 participants