Conversation
|
Size Change: +3 kB (0%) Total Size: 507 kB
ℹ️ View Unchanged
|
4acf494 to
6238d34
Compare
assets/src/edit-story/migration/migrations/test/v0009_pageAdvancement.js
Outdated
Show resolved
Hide resolved
c411863 to
c608771
Compare
935a169 to
79a29f6
Compare
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
79a29f6 to
d42af5b
Compare
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
# Conflicts: # assets/src/edit-story/migration/migrate.js
| id={id} | ||
| auto-advance-after={ | ||
| autoAdvance | ||
| ? longestMediaElement?.id || `${defaultPageDuration}s` |
There was a problem hiding this comment.
If the longest video is super short, e.g. 1s, shouldn't it use the defaultPageDuration? Generally, if the longest media is shorter than the set duration, should it use that instead?
There was a problem hiding this comment.
mhh...good point. I do think that if somebody drags in a ~4 seconds video, and the page duration is set to 7s, they will want auto advance after the video (a frozen video almost always looks strange). However, I can see the case for small, decorative 'transition in' videos.
For now, let's ship with this implementation, but paging @samitron7 to chime in, and possibly design a per-video override, e.g. a checkbox in the video settings that says something like "Decorative (does not trigger auto advancing)". We can also try a certain threshold (if video is less than 1/3 of page duration, it will not auto advance), but these all seem unsafe.
* Added defaults for scale, focalX, focalY * Fix prop types warnings for autoAdvance See #714. Co-authored-by: Pascal Birchler <pascalb@google.com>
Fixes #84.
Animations and audio elements are not covered by this PR because they are not implemented yet. Although audio elements wouldn't require any changes to the logic here.