-
Notifications
You must be signed in to change notification settings - Fork 900
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
Disabling mutually exclusive settings #1822
Conversation
…settings to disappear when appropriate
I like! Do u still need to more changes or can i approve? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just curious as to what the criteria should be for whether something should be hidden or greyed out for future reference
I would say if some setting or settings make the values of multiple other settings in the same tab irrelevant, those other settings should be hidden. In any other case the other setting(s) should be probably be grayed out. Graying out may also be favorable if hiding causes flexbox weirdness, but it's up to the discretion of the contributor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Local Test: |
8276fdb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Locally tested latest code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have actually tested the latest code and forgot to approve
Oops
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Disabling mutually exclusive settings
Pull Request Type
Please select what type of pull request this is:
Related issue
closes #1468
Description
ft-select
elements by giving them thedisabled
attribute.ft-select
,ft-button
,ft-input
, andft-toggle-switch
elements. Now makes it very visually clear that these controls are disabled. Their opacity is set to 0.4 when disabled, which is generally considered sufficiently grayed for accessibility purposes.Screenshots (if appropriate)
Testing (for code that is not small enough to be easily understandable)
Tested; all controls work fine when not mutually exclusive & vice versa.
Desktop (please complete the following information):
Edit: As of 1b58fd2: when applicable, the Invidious settings, Proxy settings, and SponsorBlock settings are now disappeared instead of grayed out. Screenshots of the older version are visible in the earliest edition of this post in the edit history.