Skip to content

Create TurboWarp Player Addon - #2682

Merged
WorldLanguages merged 29 commits into
ScratchAddons:masterfrom
TheColaber:tw
Jul 28, 2021
Merged

Create TurboWarp Player Addon#2682
WorldLanguages merged 29 commits into
ScratchAddons:masterfrom
TheColaber:tw

Conversation

@TheColaber

Copy link
Copy Markdown
Member

Resolves #164

Changes

Creates a button on project pages that lets you toggle from tw to scratch player or open a new tw tab.

Tests

This happens after seeing inside :/
image
The iframe size is not 100% fine yet

@GarboMuffin GarboMuffin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm still not fond of the idea of putting TurboWarp into an iframe. Too many hard or impossible to solve issues.

Very selfish request: make "Open tab" the default mode. Or don't. It's not really my choice to make.

Comment thread addons/turbowarp-player/userscript.js Outdated
Comment thread addons/turbowarp-player/userscript.js Outdated
Comment thread addons/turbowarp-player/userscript.js Outdated
Comment thread addons/turbowarp-player/icon.svg Outdated
Comment thread addons/turbowarp-player/userscript.js Outdated
@Shluffy Shluffy mentioned this pull request Jun 8, 2021
@TheColaber TheColaber added the scope: addon Related to one or multiple addons label Jun 13, 2021

@GarboMuffin GarboMuffin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

other feedback:

  • In replace player mode, if Scratch either Scratch or TurboWarp are running, it keeps running when mode is switched. That's not ideal.
    • For Scratch you could stop or pause the project
    • For TurboWarp you could remove the iframe from the DOM instead of just hiding it

Comment thread addons/turbowarp-player/userscript.js Outdated
Comment thread addons/turbowarp-player/userscript.js Outdated
Comment thread addons/turbowarp-player/userscript.js Outdated
Comment thread addons/turbowarp-player/userscript.js Outdated
@WorldLanguages

Copy link
Copy Markdown
Member

This happens after seeing inside :/

Will be solved by #2852

@TheColaber TheColaber added the status: pending A PR is still not ready to merge, or an issue is being worked on/on consideration label Jun 28, 2021
@FunctionalMetatable

Copy link
Copy Markdown
Contributor

Can there be the ability to load from different mods, e.g forkphorus?

@FunctionalMetatable

FunctionalMetatable commented Jul 12, 2021

Copy link
Copy Markdown
Contributor

+ For TurboWarp, could there be a setting for enabling 60fps/high quality pen?

@WorldLanguages WorldLanguages removed the status: pending A PR is still not ready to merge, or an issue is being worked on/on consideration label Jul 17, 2021
@WorldLanguages

WorldLanguages commented Jul 17, 2021

Copy link
Copy Markdown
Member

TODOs:

  • We'll keep "new tab" as the default, so I don't see the point on having auto replace. The feature should be removed altogether.
  • div.project-info-alert should be hidden when on iframe mode ("this project uses username block" or "you cannot use block data", sometimes both)
  • Let's make the iframe background a bit lighter or darker (depending on whether website dark mode is enabled) when on iframe mode. Plus border-radius.
  • Iframe sizing needs to be enhanced.
  • We should probably ask the user to confirm that they want to go inside the project when there's an iframe, since it will be destroyed (and maybe the other way, since the project will be stopped)
  • Good addon description and notices, so that it's clear that 1) addons won't work in iframe mode 2) turbowarp will get your username in iframe mode 3) it only works on player mode, not editor mode.

@daniel4-scratch

Copy link
Copy Markdown
Contributor

idea: if the project notes contains a turbowarp link with settings the button redirects to that one

Comment thread addons/turbowarp-player/userscript.js
@TheColaber
TheColaber requested a review from apple502j as a code owner July 24, 2021 20:13
@WorldLanguages

Copy link
Copy Markdown
Member

@mxmou Could you 1) tell me if addon name is OK 2) tell me if addon description is OK 3) tell me what you think about the background-color applied to .sa-tw-iframe, both in light mode and dark mode (currently both rgba(0, 0, 0, 0.1))?

@mxmou

mxmou commented Jul 25, 2021

Copy link
Copy Markdown
Member

@mxmou Could you 1) tell me if addon name is OK 2) tell me if addon description is OK 3) tell me what you think about the background-color applied to .sa-tw-iframe, both in light mode and dark mode (currently both rgba(0, 0, 0, 0.1))?

I like the title and description, but like #2799 changes the remix tree button addon, I think the project page should not be mentioned in the title - maybe add all information about the button's location to the description instead.

In notices, the "replace player" mode should probably be called the "replace player" mode, not just "replace player" mode. I'm not sure if "embed" can be used as a noun - maybe "replaces the Scratch player with an embedded TurboWarp player"?

I don't think the background is necessary, and it doesn't fit with Scratch's UI. I think it's better to put the iframe in a container with overflow: hidden and add a negative margin to align it with the rest of the page. If the background is important for some reason, a better color choice might be white in light mode and #282828 in dark mode, with a border. It could also be styled similarly to the project description, with a transparent blue background and border.

@WorldLanguages

Copy link
Copy Markdown
Member

TODO: English grammar check for notices

@WorldLanguages

Copy link
Copy Markdown
Member

English notices approved by @TheColaber and Grammarly

@WorldLanguages WorldLanguages left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Merging tomorrow

@daniel4-scratch

Copy link
Copy Markdown
Contributor

Merging tomorrow

Nice

@mxmou

mxmou commented Jul 28, 2021

Copy link
Copy Markdown
Member

This iframe background looks good, but I still think there should be at least an option to make the TurboWarp stage the same size as Scratch's, with no background.

@retronbv

Copy link
Copy Markdown
Contributor

Merging tomorrow

Frick yea my addons idea is being added

@WorldLanguages

Copy link
Copy Markdown
Member

This iframe background looks good, but I still think there should be at least an option to make the TurboWarp stage the same size as Scratch's, with no background.

I don't think there's a way to do that. Keep in mind the margin you see between the stage and the border is not artificially added by this addon. It's margin inside the iframe. So we can't make it smaller.
We do make the iframe 2px less wide and 2px less tall for the border, tho. But we don't add any more margins. We can't keep the stage the exact same size.

@WorldLanguages
WorldLanguages merged commit bc3daea into ScratchAddons:master Jul 28, 2021
@mxmou

mxmou commented Jul 28, 2021

Copy link
Copy Markdown
Member

This iframe background looks good, but I still think there should be at least an option to make the TurboWarp stage the same size as Scratch's, with no background.

I don't think there's a way to do that. Keep in mind the margin you see between the stage and the border is not artificially added by this addon. It's margin inside the iframe. So we can't make it smaller.
We do make the iframe 2px less wide and 2px less tall for the border, tho. But we don't add any more margins. We can't keep the stage the exact same size.

We can. I'll make a PR when I have time.

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

Labels

scope: addon Related to one or multiple addons

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Turbowarp player in editor (player)

8 participants