Skip to content

fix(snippets): update full screen snippets selectors#1049

Merged
kyrie25 merged 4 commits intospicetify:mainfrom
D3SOX:patch-1
Nov 25, 2025
Merged

fix(snippets): update full screen snippets selectors#1049
kyrie25 merged 4 commits intospicetify:mainfrom
D3SOX:patch-1

Conversation

@D3SOX
Copy link
Copy Markdown
Contributor

@D3SOX D3SOX commented Nov 25, 2025

Was thinking of changing it to use [data-testid="fullscreen-mode-button"] instead but I'm unsure if these are always set. Do you have an opinion on this?

@D3SOX D3SOX requested a review from a team as a code owner November 25, 2025 11:48
@D3SOX D3SOX requested review from rxri and removed request for a team November 25, 2025 11:48
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 25, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Updated two CSS snippet entries in resources/snippets.json: both Full Screen button selectors were replaced with [data-testid="fullscreen-mode-button"] variants and their display: none rules adjusted; no logic or control-flow changes.

Changes

Cohort / File(s) Summary
CSS snippet edits
resources/snippets.json
Two CSS snippet entries modified: one selector changed to [data-testid="fullscreen-mode-button"] { display: none; } (removed !important), the other to [data-testid="fullscreen-mode-button"]:not(`#BeautifulLyricsFullscreenButton`) { display: none !important; }. No other edits.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Small selector-only changes in a single JSON file.
  • Review attention:
    • Confirm the data-testid selector matches the intended DOM element.
    • Verify the differing use of !important is deliberate and doesn't change behavior unexpectedly.

Suggested reviewers

  • rxri

Poem

🐇 I hopped through CSS and found a thread,
A hidden button tucked in red.
I switched a selector, soft and light,
Now fullscreen sleeps away from sight. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title accurately describes the main change: updating CSS selectors for fullscreen button snippets from path-based selectors to data-testid attributes.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c35c00 and ba7938e.

📒 Files selected for processing (1)
  • resources/snippets.json (2 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@D3SOX D3SOX changed the title fix: update hide full screen button snippet to new icon fix(snippets): update hide full screen button to new icon Nov 25, 2025
Copy link
Copy Markdown
Member

@kyrie25 kyrie25 left a comment

Choose a reason for hiding this comment

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

Was thinking of changing it to use [data-testid="fullscreen-mode-button"] instead but I'm unsure if these are always set. Do you have an opinion on this?

You should, and it is definitely better than using SVG path as a selector.

@D3SOX D3SOX requested a review from kyrie25 November 25, 2025 13:28
@kyrie25 kyrie25 changed the title fix(snippets): update hide full screen button to new icon fix(snippets): update Hide Full Screen Button selector Nov 25, 2025
@kyrie25 kyrie25 changed the title fix(snippets): update Hide Full Screen Button selector fix(snippets): update full screen snippets selectors Nov 25, 2025
@kyrie25 kyrie25 merged commit 92990c3 into spicetify:main Nov 25, 2025
4 of 5 checks passed
@D3SOX D3SOX deleted the patch-1 branch November 25, 2025 13:52
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