Skip to content

Conversation

@ameliasquires
Copy link

added a slider similar to force styling blacklist, with very similar functionality, but for non-forced styles.

implementation is similar to how the blacklist for force styling works, but now exits earlier where the global enable styling would've exited.

made this for fun, and because i prefer the styling on certain websites more than others.

image

@ameliasquires
Copy link
Author

i also added some hostname normalization in popup/popup.js:saveSkipForceThemingList that may have caused some issues

@sameerasw sameerasw requested review from Copilot and sameerasw April 28, 2025 22:20
@sameerasw sameerasw added the enhancement New feature or request label Apr 28, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a new whitelist/blacklist feature for styling, providing an additional slider control to selectively enable or disable non-forced styling per website. Key changes include:

  • Adding new UI elements and event listeners in popup.js and popup.html for whitelist styling mode and skip theming.
  • Updating storage logic for managing the skip theming list.
  • Modifying background.js to incorporate the new whitelist styling mode logic in early exit conditions.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
popup/popup.js Introduces new DOM elements, settings, and methods to support whitelist styling mode and skip theming list.
popup/popup.html Adds corresponding UI elements for user interaction with whitelist styling mode and skip theming.
background.js Updates the logic in applyCSSToTab to use the new whitelist styling mode in controlling CSS injection.
Comment on lines +242 to +243
!styleMode && skipStyleList.includes(hostname) ||
styleMode && !skipStyleList.includes(hostname)) {
Copy link

Copilot AI Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditional logic here could be misinterpreted due to operator precedence. Adding parentheses to clearly group the conditions would improve readability and reduce potential logical errors.

Suggested change
!styleMode && skipStyleList.includes(hostname) ||
styleMode && !skipStyleList.includes(hostname)) {
(!styleMode && skipStyleList.includes(hostname)) ||
(styleMode && !skipStyleList.includes(hostname))) {
Copilot uses AI. Check for mistakes.
Comment on lines +277 to +278
const data = await browser.storage.local.get(SKIP_THEMING_KEY);
this.skipThemingList = data[SKIP_THEMING_KEY] || [];
Copy link

Copilot AI Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding error handling for the storage.get promise in loadSkipThemingList to ensure any retrieval issues are caught and logged, similar to other async operations in the extension.

Suggested change
const data = await browser.storage.local.get(SKIP_THEMING_KEY);
this.skipThemingList = data[SKIP_THEMING_KEY] || [];
try {
const data = await browser.storage.local.get(SKIP_THEMING_KEY);
this.skipThemingList = data[SKIP_THEMING_KEY] || [];
} catch (error) {
console.error("Failed to load skipThemingList:", error);
this.skipThemingList = [];
}
Copilot uses AI. Check for mistakes.
@sameerasw sameerasw merged commit 1ede58e into sameerasw:master Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

2 participants