Skip to content

Conversation

@exoego
Copy link
Contributor

@exoego exoego commented Mar 23, 2020

Closes #376

Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

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

Thanks! I have two comments:

var onfullscreenerror: js.Function1[Event, _] = js.native
}

class FullscreenOptions(
Copy link
Member

Choose a reason for hiding this comment

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

According to MDN, this should be a trait with a var navigationUI instead of a class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 5147c02

* MDN
*/
def requestFullscreen(
options: FullscreenOptions = ???): js.Promise[Unit] = js.native
Copy link
Member

Choose a reason for hiding this comment

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

Prefer js.native instead of ??? for default values in JS APIs:

Suggested change
options: FullscreenOptions = ???): js.Promise[Unit] = js.native
options: FullscreenOptions = js.native): js.Promise[Unit] = js.native
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 075d58a

@sjrd sjrd changed the title Add Fullscreen API Fix #376: Add the Fullscreen API Mar 24, 2020
@sjrd sjrd merged commit fb9072f into scala-js:master Mar 24, 2020
@exoego exoego deleted the fullscreen branch March 24, 2020 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants