[ASK DAN BEFORE MERGING, expected mid-Jan] Maker deprecation warnings - #49419
Merged
Conversation
bencodeorg
commented
Dec 9, 2022
| render() { | ||
| // Experiment 'microbit', displays Circuit Playground and Micro:Bit descriptions. | ||
| let isMicrobit = experiments.isEnabled('microbit'); | ||
| const isMicrobit = experiments.isEnabled('microbit'); |
Contributor
Author
There was a problem hiding this comment.
Unrelated cleanup.
molly-moen
reviewed
Dec 12, 2022
| return ( | ||
| <div> | ||
| {isCodeOrgBrowser() && ( | ||
| <Notification |
Contributor
There was a problem hiding this comment.
should we have two banners here? Is it possible to combine these into one message?
Contributor
Author
There was a problem hiding this comment.
I don't think anyone would see both...the first is for the Maker app, and the second for old chrome versions? Unless I'm misunderstanding something
Contributor
There was a problem hiding this comment.
ah ok I was confused because your screenshot had both
Contributor
Author
There was a problem hiding this comment.
ah apologies, was trying to minimize screenshot screen real estate -- should have split those out!
molly-moen
approved these changes
Dec 12, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds deprecation warnings to Maker setup and homepages.
/maker/setup (first notification visible to Maker app users, second notification visible to old Chrome versions. No one should see both notifications)
/maker/home (notification visible to everyone)
Links
Testing story
Tested manually that the Chrome version detection worked for me locally. I did not test the Maker app (I don't have it set up locally, and the code I'm adding seems pretty straightforward).
Deployment strategy
This should be merged as part of Maker deprecation messaging in January -- check in with Dan for details.