Skip to content

Add a helper function to check if GUI is running in Scratch Desktop#4113

Merged
cwillisf merged 5 commits intoscratchfoundation:developfrom
cwillisf:isScratchDesktop
Dec 19, 2018
Merged

Add a helper function to check if GUI is running in Scratch Desktop#4113
cwillisf merged 5 commits intoscratchfoundation:developfrom
cwillisf:isScratchDesktop

Conversation

@cwillisf
Copy link
Contributor

@cwillisf cwillisf commented Dec 17, 2018

Resolves

Resolves scratchfoundation/scratch-desktop#12

Proposed Changes

This adds a function, isScratchDesktop, which returns true when run from inside Scratch Desktop (or any other Electron host) and false otherwise. This new function can be overridden manually by setting an override value, or by calling an associated function which can parse window.location.href to extract an override value if present.

This change also includes an example of using isScratchDesktop to exclude tutorial decks which require a project by ID, since those won't be possible to follow without an Internet connection.

Reason for Changes

Scratch Desktop has some special requirements of the GUI, such as enabling or disabling certain features or labeling some menus and buttons differently.

I would just do this in my special scratch-desktop branch of scratch-gui, but I want to make the helper function available for #3964 and any other similar upcoming work.

Test Coverage

Currently the testing process is manual, but it's possible to test through the playground by adding ?isScratchDesktop=1 to the playground URL.

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Mac

  • Chrome
  • Firefox
  • Safari

Windows

  • Chrome
  • Firefox
  • Edge

Chromebook

  • Chrome

iPad

  • Safari

Android Tablet

  • Chrome

Copy link
Contributor

@rschamp rschamp left a comment

Choose a reason for hiding this comment

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

Maybe this should just be a prop we pass in? This is a bit of guesswork and engineering for something we can tell GUI definitively. And Scratch Desktop can also pass in props like the one to disable the backpack.

@cwillisf
Copy link
Contributor Author

@rschamp I'm cool with adding it as a prop but I'm not completely sure how to make it work.

For example, how would you recommend handling the changes I made in src/lib/libraries/decks/index.jsx? Note that decks/index.jsx is imported from at least 5 different places in scratch-gui (some of which are tests).

If there's a good way to handle that then yeah, I'd definitely prefer an externally-driven approach!

@cwillisf
Copy link
Contributor Author

cwillisf commented Dec 18, 2018

From discussion with @rschamp just now, the new plan is:

  1. Add a prop on GUI
  2. Use the prop in a way that's similar to the current override mechanism
  3. Bias toward using the value near render-time instead of early to avoid the possibility of trying to check isScratchDesktop before the prop has been processed

The tutorial deck filter will probably move out of src/lib/libraries/decks/index.jsx and into the code which consumes that data. This does mean that some use-cases of the tutorial library may be unfiltered but that should be OK for any case which doesn't involve rendering every item. It looks like there's only one "customer" that renders every item so it should be OK to just filter there.

Changes coming soon...

@cwillisf
Copy link
Contributor Author

I added notScratchDesktop since I found that most of my checks were actually phrased in terms of !scratchDesktop(). Depending on the context, eslint objects to the negation which leads to listing the desktop case first, before the normal non-desktop case, which IMO feels wrong.

Christopher Willis-Ford added 2 commits December 19, 2018 08:49
This was intended as a safety check but it causes problems with some
tests.
@rschamp rschamp assigned cwillisf and unassigned paulkaplan and rschamp Dec 19, 2018
@cwillisf cwillisf merged commit 0544641 into scratchfoundation:develop Dec 19, 2018
@cwillisf cwillisf deleted the isScratchDesktop branch December 19, 2018 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants