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

web: Convert demo to typescript and copy the new extension-player styling over #14093

Merged
merged 10 commits into from
Nov 21, 2023

Conversation

Dinnerbone
Copy link
Contributor

The demo now has the improvements from #13473, and uses the actual API through typescript

@Dinnerbone Dinnerbone force-pushed the demo branch 2 times, most recently from b8d4d29 to 1166862 Compare November 21, 2023 16:57
const overlay = document.getElementById("overlay");
const authorContainer = document.getElementById("author-container");
const author = document.getElementById("author");
const main = document.getElementById("player-container")!;
Copy link
Contributor

Choose a reason for hiding this comment

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

In https://github.com/ruffle-rs/ruffle/blob/master/web/packages/extension/src/player.ts#L25, this was renamed playerContainer. That name is probably better since that matches the id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I renamed it back just to help work with the diff, but I've renamed it again now

const main = document.getElementById("player-container")!;
const overlay = document.getElementById("overlay")!;
const authorContainer = document.getElementById("author-container")!;
const author = <HTMLLinkElement>document.getElementById("author");
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to use the prefix type cast operator instead of as (here and elsewhere)? The as operator is preferred according to https://www.typescriptlang.org/docs/handbook/release-notes/typescript-1-6.html#new-tsx-file-extension-and-as-operator

Copy link
Contributor Author

@Dinnerbone Dinnerbone Nov 21, 2023

Choose a reason for hiding this comment

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

I'll fix it here and add a lint in a future PR to enforce as, as we use a lot of angle casting currently

Copy link
Contributor

@danielhjacobs danielhjacobs left a comment

Choose a reason for hiding this comment

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

This code is needed to make the "Select File" button work:

document
.getElementById("local-file-label")!
.addEventListener("click", () => {
document.getElementById("local-file")!.click();
});

@danielhjacobs
Copy link
Contributor

The location of the Ruffle logo is not ideal on small screens (note: this is this PR with the URL input parameter set to true to make the screen as crowded as possible).

image

@Dinnerbone
Copy link
Contributor Author

Made it hide the logo when the screen is <=600px wide

@Dinnerbone Dinnerbone merged commit 5177bd4 into ruffle-rs:master Nov 21, 2023
13 checks passed
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.

2 participants