discuss-button: support new Membership link #8676
Conversation
mxmou
added
scope: addon
There was a problem hiding this comment.
The old code crashes on www pages because:
- When the page is initially loaded, the Membership link is shown.
- The addon runs and removes all the links, replacing them with new elements.
- Then, when the session data is loaded, React tries to remove the Membership link. Since the element has already been removed by the addon, this causes a crash.
I've rewritten it to override the navbar's render function instead of modifying the DOM directly.
There was a problem hiding this comment.
What kind of crash does the old code trigger? Does the page stop working?
There was a problem hiding this comment.
What kind of crash does the old code trigger? Does the page stop working?
It shows the full-screen "Scratch has crashed" message.
There was a problem hiding this comment.
I can't repro with just discuss-button or all addons enabled.
DNin01
added
the
priority: 1
|
We'll need to decide what to do with existing users' settings: should they be kept (in that case, the new setting should be turned off for them) or should the About item be removed? |
I've moved them to a separate PR since they're no longer needed here.
|
Please ping me when this is done so that we can release a new version |
I quickly added a setting update: if after the update the addon settings contain an item with a URL of "/about", it is removed. (If we release this patch before Scratch adds the Membership button, then if a user has this addon enabled, that button will appear on their nav bar early. It'll also go to a "not found" page, but that's better than crashing the site.) |
|
@WorldLanguages, I think the PR is ready. Things to cherry-pick for this next hotfix (v1.44.2): |
* discuss-button: support new Membership link * Separate "Show Membership link" setting * Revert settings page changes I've moved them to a separate PR since they're no longer needed here. * Migrate discuss-button settings --------- Co-authored-by: DNin01 <106490990+DNin01@users.noreply.github.com>
|
@WorldLanguages I've already done the cherry-picking and prepared the next hotfix on branch |
5 participants
Changes
Scratch will remove the About link from the navigation bar and add a new Membership link instead, which will only be visible when logged out. This PR updates the customizable navigation bar addon accordingly:
Tests
Tested locally on Edge.