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

XKit Preferences: Remove broken Old Sidebar script #2116

Merged
merged 2 commits into from
Jul 22, 2023

Conversation

marcustyphoon
Copy link

Old Sidebar, of course, doesn't work, but more to the point it still does run this, which is bad:

var account = document.getElementById("account_button");
account.click();

This removes it (without a notification, to avoid user confusion due to an update to New XKit containing the phrase "sidebar" coming during a period of discussion about a related-but-different sidebar).

@AprilSylph
Copy link
Member

Not sure if we care enough to also remove it from the non-React version of the "Get Extensions" tab.

Also, will give @nightpool the honour of the final say between doing this and not.

@nightpool
Copy link
Member

we could ask @estufar

if it's spring cleaned it won't function anyway
@marcustyphoon
Copy link
Author

marcustyphoon commented Jul 20, 2023

Not sure if we care enough to also remove it from the non-React version of the "Get Extensions" tab.

Ah, yeah, good point. To the extent that we still support non-React anything, this PR may as well not enable an "install extension but it immediately silently vanishes" interaction.

@nightpool
Copy link
Member

Lived with this installed for 2 days and it was annoying enough that i've decided we should remove it, although i still like the concept and it would be fun to update it to modern react.... my only concern is that if we get rid of it now it might be annoying to add back but it's been broken long enough i think it makes sense to do the silent removal for now and then people can re-add it when it works again (or we can just add it to xrw or whatever)

@nightpool nightpool merged commit 70dee2b into new-xkit:master Jul 22, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants