Skip to content

Add Disable Lyrics Not Found Notice option - #316

Open
Eloren1 wants to merge 3 commits into
Spikerko:mainfrom
Eloren1:disable-lyrics-not-found
Open

Add Disable Lyrics Not Found Notice option#316
Eloren1 wants to merge 3 commits into
Spikerko:mainfrom
Eloren1:disable-lyrics-not-found

Conversation

@Eloren1

@Eloren1 Eloren1 commented Apr 10, 2026

Copy link
Copy Markdown

Add an option to disable a Discord ad notice that makes me skip songs without lyrics more often which is a bad UX.

image

@greptile-apps

greptile-apps Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a "Disable Lyrics Not Found Notice" toggle to the settings menu, storing the preference and applying a CSS class (sl_hide_lyrics_not_found_notice) to hide the notice. The logic in app.tsx and settings.ts is correct, but the CSS rule that does the actual hiding is broken due to a brace-placement mistake.

  • P1 — src/css/default.scss: The new rule is nested inside body.sl_settings_top, compiling to body.sl_settings_top body.sl_hide_lyrics_not_found_notice ... — an impossible descendant selector that never matches. The notice is never hidden on page load even when the toggle is enabled.

Confidence Score: 4/5

Not safe to merge — the single CSS bug makes the core feature completely non-functional on page load.

One P1 finding remains: the SCSS nesting error renders the hiding rule dead CSS, so the feature never works after a page reload. The fix is a one-line brace placement change.

src/css/default.scss — the new rule must be moved outside the body.sl_settings_top block.

Important Files Changed

Filename Overview
src/css/default.scss New body.sl_hide_lyrics_not_found_notice rule is accidentally nested inside body.sl_settings_top, producing an unmatchable descendant selector — the hide logic is broken CSS.
src/app.tsx Initialization and application of DisableLyricsNotFoundNotice follows the existing pattern correctly; class toggling logic is sound.
src/components/Global/Defaults.ts Adds DisableLyricsNotFoundNotice: false default — straightforward and consistent with the existing pattern.
src/utils/settings.ts Toggle registration and callback correctly treat getFieldValue return value as a boolean and store the proper "true"/"false" strings; no string-vs-boolean comparison bug.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[app.tsx startup] --> B{storage has disableLyricsNotFoundNotice?}
    B -- No --> C[Set to 'false']
    C --> D[Read value]
    B -- Yes --> D
    D --> E{value === 'true'?}
    E -- Yes --> F[Defaults.DisableLyricsNotFoundNotice = true]
    E -- No --> G[Defaults.DisableLyricsNotFoundNotice = false]
    F --> H[body.classList.add sl_hide_lyrics_not_found_notice]
    G --> I[body.classList.remove sl_hide_lyrics_not_found_notice]
    H --> J[CSS applies? ❌ Broken selector - nested inside body.sl_settings_top]
    I --> K[Notice visible ✓]
    L[User toggles setting] --> M[getFieldValue as boolean]
    M --> N[storage.set 'true' or 'false']
    N --> O[classList.toggle sl_hide_lyrics_not_found_notice]
    O --> J
Loading

Reviews (2): Last reviewed commit: "Update src/css/default.scss" | Re-trigger Greptile

Comment thread src/utils/settings.ts Outdated
Comment thread src/css/default.scss Outdated
Eloren1 and others added 2 commits April 10, 2026 12:05
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@Eloren1

Eloren1 commented Apr 10, 2026

Copy link
Copy Markdown
Author

@greptileai

@greptile-apps

greptile-apps Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Tip:

Greploop — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

@ReddRubinho

Copy link
Copy Markdown

On god i need this

@Eloren1

Eloren1 commented Jun 12, 2026

Copy link
Copy Markdown
Author

Marketplace → Snippets → Add CSS

.notice-footer, .notice-descriptor { display: none !important; }

Should be an option...

@Eloren1

Eloren1 commented Jun 12, 2026

Copy link
Copy Markdown
Author

@Spikerko I join servers I'm interested in, but this thing only adds frustration, makes me skip songs unwittingly, and takes away any desire to join. A button in the settings with a Discord link would be sufficient and even more effective.

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.

2 participants