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

Add SpriteLab entrypoint #29831

Merged
merged 3 commits into from Jul 22, 2019
Merged

Add SpriteLab entrypoint #29831

merged 3 commits into from Jul 22, 2019

Conversation

ajpal
Copy link
Contributor

@ajpal ajpal commented Jul 22, 2019

Third attempt at creating this entrypoint (#29550 was reverted by #29707, then #29731 was reverted by #29790)

This PR addresses the issue with #29731 by adding a case for appOptions.app === 'spritelab' in all of the places we check appOptions.app === 'gamelab'. As future work, we'd like to get rid of these hard-coded app type checks at some point.

@@ -17,10 +17,7 @@ export var checkForUnsupportedBrowsersOnLoad = function() {
if (isMobileDevice()) {
if (appOptions.app === 'applab') {
textDivId = '#applab-unsupported-tablet';
} else if (
appOptions.app === 'gamelab' &&
!appOptions.valueTypeTabShapeMap
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the reason for removing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal here was to show the warning for gamelab but not spritelab (valueTypeTabeShapeMap is only set for spritelab). Now that spritelab is its own app type, though, this check can just be appOptions.app === 'gamelab'

Copy link
Contributor

Choose a reason for hiding this comment

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

ugh... valueTypeTabShapeMap identifying spritelab.

Thanks for catching that!

Copy link
Contributor

@jmkulwik jmkulwik left a comment

Choose a reason for hiding this comment

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

LGTM!

@ajpal ajpal merged commit b7ed99d into staging Jul 22, 2019
@ajpal ajpal deleted the jul18-spritelab branch July 25, 2019 01:20
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.

None yet

2 participants