separate legacyShareStyle from isLegacyShare #22615
Conversation
| @@ -523,7 +523,7 @@ var projects = module.exports = { | |||
| this.sourceHandler.setInitialLevelSource(currentSources.source); | |||
| this.showMinimalProjectHeader(); | |||
| } | |||
| } else if (appOptions.isLegacyShare && this.getStandaloneApp()) { | |||
| } else if (appOptions.legacyShareStyle && this.getStandaloneApp()) { | |||
There was a problem hiding this comment.
This is the only one I'm not certain on. Are we trying to determine if we're a /c share URL here? We're outside of the this.isProjectLevel() check. One reason to choose legacyShareStyle is that it is true in all of the places where we previously had set isLegacyShare to true - so I'm not changing any behavior.
There was a problem hiding this comment.
The comment on showMinimalProjectHeader() reads
// Minimal project header for viewing channel shares and legacy /c/ share pages.which I think means you've got it right - we want the minimal header in both cases.
| // and playlab). So instead, we check the first letter (after the /) in the path name of the | ||
| // url, as legacy shares all start with /c | ||
| // var isLegacyShare = this.props.isLegacyShare; | ||
| var isLegacyShare = window.location.pathname[1] === 'c'; |
There was a problem hiding this comment.
This code had noticed that something was wrong (the comment says isLegacyShare currently has the wrong value) - but the fix was hacky and narrow in scope.
| view_options(no_header: true, no_footer: true, code_studio_logo: true) | ||
| @is_legacy_share = true | ||
| view_options(no_header: true, no_footer: true, code_studio_logo: true, is_legacy_share: true) | ||
| @legacy_share_style = true |
There was a problem hiding this comment.
In this controller, we set both is_legacy_share and legacy_share_style
| @@ -300,7 +300,7 @@ def show | |||
| # for sharing pages, the app will display the footer inside the playspace instead | |||
| no_footer = sharing | |||
| # if the game doesn't own the sharing footer, treat it as a legacy share | |||
| @is_legacy_share = sharing && !@game.owns_footer_for_share? | |||
| @legacy_share_style = sharing && !@game.owns_footer_for_share? | |||
There was a problem hiding this comment.
In the projects controller, we only ever set legacy_share_style (in some cases)
|
Quick inventory of things now determined by legacyShareStyle:
I wonder if we don't want to use the "legacy share" description for this at all, to avoid confusing this with |
|
Correct, I believe it is everything except AppLab/WebLab, though WebLab now bypasses this share mechanism completely and shares via codeprojects.org. My sense is that we thought we might migrate more apps over to the newer "applab" style, but never got around to it. I thought about choosing a different name, but since we have classes like |
/cURL share, meaning that it referenced a level source id and not a project idlegacy_share_style(akalegacyShareStyle) for the former and sending the existingis_legacy_share(akaisLegacyShare) for the latter. As we currently stand,isLegacyShareandlegacyShareStylecan both be set at the same time, orlegacyShareStylecan be true by itself.isLegacySharein theappscode to choose the appropriate variable to key off of.