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.

Sign up for GitHub

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

Jump to bottom

web: Add ruffle() (Version 1) API #17939

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

web: Add ruffle() (Version 1) API #17939

wants to merge 6 commits into from

Conversation

Dinnerbone
Copy link
Contributor

Bikeshedding time, here's the chance to decide what the "v1 api" looks like before we lock it in.

The only change so far is that I've added a specific method for calling EI, vs the very easy-to-break and implicit "the method will exist, trust me". I'm awaiting feedback before touching anything else.

Whilst this is "version 1" (PlayerV1 interface), if we never add a V2 that's totally fine. ruffle() is the same as ruffle(1).

Things to consider for each existing member of the API:

  • Is anything a property that should be a method?
  • Is anything a method that should be a property?
  • Should we rename anything, or change arguments?

@Dinnerbone Dinnerbone added A-web Area: Web & Extensions T-feature Type: New Feature (that Flash doesn't have) labels Sep 15, 2024
Dinnerbone
Dinnerbone commented Sep 15, 2024
View reviewed changes
web/packages/core/src/public/v1.ts Outdated
*
* @param observer The observer that will be called for each trace.
*/
set traceObserver(observer: ((message: string) => void) | null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want to set the trace observer, as today - or set a trace observer?

@Dinnerbone
Copy link
Contributor Author

I've gone ahead and changed pause()/play()/isPlaying to suspend()/resume()/suspended as that seems fairly agreed upon. Comment if anyone disagrees with the specifics before this PR is finished.

@Dinnerbone Dinnerbone mentioned this pull request Oct 2, 2024
Merged
@Dinnerbone
Copy link
Contributor Author

Renamed enterFullscreen to requestFullscreen, and removed setFullscreen.
Decided to keep fullscreenEnabled as if we still support old ios, it's likely users will forget to check the prefixed stuff too.

@Dinnerbone
Copy link
Contributor Author

Changed onFSCommand (getter/setter) to addFSCommandHandler (method).

@Dinnerbone
Copy link
Contributor Author

Made ReadyState a string

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

@evilpie evilpie evilpie left review comments

@danielhjacobs danielhjacobs danielhjacobs left review comments

Assignees
No one assigned
Labels
A-web Area: Web & Extensions newsworthy T-feature Type: New Feature (that Flash doesn't have)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Dinnerbone @evilpie @danielhjacobs @n0samu