Skip to content

Use aria-label for Preferences selector - #2181

Merged
mquevill merged 4 commits into
sindresorhus:mainfrom
mquevill:UpdatePreferencesSelector
Nov 30, 2024
Merged

mquevill merged 4 commits into
sindresorhus:mainfrom
mquevill:UpdatePreferencesSelector

Conversation

@mquevill

Copy link
Copy Markdown
Collaborator

The selector that was previously used is also used for Security Alerts (including entering in your message sync PIN) and other windows, so it would be difficult to use that style of selector. Using aria-label=Preferences seems to be the best way to address this.

The current method used elementReady() on the first element that matched, which was usually the Sync PIN window, which caused some other conflicts with how the typical code flow would work.

The extra "style" code is no longer necessary with the backend design changes. (It's probably been outdated for some time...)

Fixes #2167.

@mquevill
mquevill requested a review from dusansimic June 18, 2024 01:46
@logiclrd

logiclrd commented Jul 8, 2024

Copy link
Copy Markdown

In #2167, there is a comment indicating that the reason the security dialog doesn't show up is that the root element has class hide-preferences-window. Empirically, I can confirm that removing this class makes the security dialog show up. This PR is marked as fixing #2167, but won't hide-preferences-window still be on the root element??

@mquevill

mquevill commented Jul 8, 2024 •

Copy link
Copy Markdown
Collaborator Author

In #2167, there is a comment indicating that the reason the security dialog doesn't show up is that the root element has class hide-preferences-window. Empirically, I can confirm that removing this class makes the security dialog show up. This PR is marked as fixing #2167, but won't hide-preferences-window still be on the root element??

The hide-preferences-window class affects all pop-ups (preferences window, sync messages window, etc.) and should be removed after the hidden preferences window is closed. The issue has been that the function isPreferencesOpen() would return true even if the "Sync now" dialog was open. By switching to the aria-label, it's more specific to the preferences window only. This means that the code will correctly detect when that window closes and the hide-preferences-window class can be removed.

@mquevill
mquevill force-pushed the UpdatePreferencesSelector branch from 66bacbd to d804634 Compare November 29, 2024 22:57
Now with updated main from upstream
@semikolon

Copy link
Copy Markdown

I just upgraded to 2.60.2 and it does not successfully remove the "hide-preferences-window" class on the html tag. Don't know why. Messenger is set to English if it matters.

@vibl

vibl commented Dec 2, 2024 •

Copy link
Copy Markdown

I just upgraded to 2.60.2 and it does not successfully remove the "hide-preferences-window" class on the html tag. Don't know why. Messenger is set to English if it matters.

Same for 2.60.3 here.

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.

"Messages are missing. Sync now." does nothing

4 participants