Skip to content

refactor: More centralization of "Audio Effect" code#505

Merged
cyanChill merged 9 commits into
devfrom
refactor/audio-module-movement
Jun 5, 2026
Merged

refactor: More centralization of "Audio Effect" code#505
cyanChill merged 9 commits into
devfrom
refactor/audio-module-movement

Conversation

@cyanChill

@cyanChill cyanChill commented Jun 5, 2026

Copy link
Copy Markdown
Member

Why

A quick overview of the changes:

  • For consistency, we now show show +0 instead of 0 on the Equalizer sliders.
  • Moved ~/modules/equalizer code into ~/modules/audio/equalizer.
  • Equalizer setting now show up in the Audio Effects screen, following a the design used for the ReplayGain settings.
  • The Playback Options sheet now shows a Audio Effects button instead of the Equalizer button.
    • Navigating to the Audio Effects screen this way will reveal additional settings (currently the Playback Speed settings is shown this way).
  • Fixed issue where marquee was visible on <Miniplayer /> component when it was pressed.

We plan for the Audio Effects screen to contain all the settings. Some of these settings will only show up when we navigate to it from the "Now Playing" screen. This method saves a couple of steps if the user wanted to manipulate multiple settings while on the "Now Playing" screen (previously would require open the sheet multiple times).

Checklist

  • Documentation is up to date to reflect these changes.
  • Ensure dependency licenses are up-to-date by running pnpm sync:licenses.
  • This diff will work correctly for pnpm android:prod.

Summary by CodeRabbit

  • New Features

    • Added Pre-Amp slider controls for Replay Gain.
  • Improvements

    • Introduced a unified Audio Effects screen (Equalizer, Replay Gain, Playback Speed).
    • Moved Playback Speed from a detached sheet to an inline setting.
    • Equalizer screen integrated into Audio Effects; navigation updated.
    • EQ graph measurement improved for more stable rendering.
    • Mini-player marquee color now responds to press state.
    • Audio Effects navigation icon updated.

cyanChill added 8 commits June 4, 2026 21:13
- For consistency as we show the `+` at 0 for the Pre-amp slider
- Follows the same idea as the "ReplayGain Pre-amp" feature.
- This bugged me a bit and the cause was due to us changing the background color on press, but not the marquee color.
- It'll only show up in the "Audio Effects" screen if we navigate here via the "Playback Options" sheet.
@cyanChill cyanChill added the Housekeeping Code maintenance or refactor. label Jun 5, 2026
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8b29eb51-7dcb-484c-9e46-e00888826f64

📥 Commits

Reviewing files that changed from the base of the PR and between 18b876d and 6df987e.

📒 Files selected for processing (1)
  • mobile/src/navigation/screens/now-playing/sheets/PlaybackOptionsSheet.tsx

📝 Walkthrough

Walkthrough

Consolidates audio settings: makes AudioEffects screen route-param-aware and conditionally shows hidden controls, moves playback speed into the segmented list, extracts Equalizer and PreAmp sliders into components, refactors EQ graph width to context, and updates navigation and icon usage.

Changes

Audio Effects Screen and Component Refactoring

Layer / File(s) Summary
Audio Effects Screen Definition
mobile/src/modules/audio/_screens.tsx
AudioEffectsView now accepts route params with showHidden?: boolean and conditionally renders PlaybackSpeedSetting when enabled, preserving EqualizerSettings and ReplayGainSettings.
Equalizer Component and EQ Graph
mobile/src/modules/audio/equalizer/components/EqualizerSettings.tsx, EQGraph.tsx, FrequencySlider.tsx
EqualizerSettings implemented with store selectors, EQ toggle, graph, per-band sliders, and presets; EQGraph measures width via onLayout and provides it through GraphWidthContext; EQLine, GraphAnnotations, GraphLabels consume the context; FrequencySlider adds trackColor and adjusts sign display.
Playback Speed Settings UI Conversion
mobile/src/modules/audio/_components/PlaybackSpeedSetting.tsx
Playback speed moved from a DetachedSheet to a SegmentedList.CustomItem rendering label, CachedSlider, and presets inline; removed local drag-state and updated preset button styling and slider options (trackColor, overlay config).
ReplayGain Pre-Amp Component Extraction
mobile/src/modules/audio/replayGain/components/PreAmpSlider.tsx, ReplayGainSettings.tsx
Introduced PreAmpSlider component selecting pre-amp by field and wiring tag-aware/tag-free update actions; ReplayGainSettings now uses PreAmpSlider with field props and imports toggleStatus.
Navigation Routes and Entry Point Updates
mobile/src/navigation/routes.tsx, mobile/src/navigation/screens/now-playing/sheets/PlaybackOptionsSheet.tsx, mobile/src/navigation/screens/settings/ExperimentalSettingsView.tsx, mobile/src/resources/icons/Equalizer.tsx
Swapped inline equalizer screen for screen-group imports, removed Equalizer icon and usages (replaced with GraphicEQ), updated PlaybackOptionsSheet to navigate to AudioEffects with { showHidden: true }, and adjusted ExperimentalSettingsView navigation call.
Supporting Changes
mobile/src/modules/scanning/hooks/useSetup.ts, mobile/src/navigation/components/MiniPlayer.tsx
Reformatted equalizer action imports into multi-line destructuring; updated MiniPlayer marquee color to switch based on isPressed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • MissingCore/Music#497: Overlaps FrequencySlider changes (animated reaction vs. live value handling) and may conflict at that component.
  • MissingCore/Music#504: Introduced the ReplayGain-oriented Audio Effects screen group that this PR extends by changing route params and conditional component rendering.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: centralizing audio effect code by moving equalizer into the audio module and consolidating settings into a unified Audio Effects screen.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@mobile/src/modules/audio/_screens.tsx`:
- Around line 11-15: The component AudioEffectsView destructures showHidden from
route.params unguarded which will throw if route or params is undefined; update
the render to safely access the optional param (e.g., read route first and then
compute showHidden with optional chaining/default: use route?.params?.showHidden
?? <safe-default>) or provide a default params object before destructuring so
that references to route.params and showHidden inside AudioEffectsView are
always safe.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 65fac5bf-7941-4933-bf96-eb0f37bd10c7

📥 Commits

Reviewing files that changed from the base of the PR and between e6bd010 and 18b876d.

📒 Files selected for processing (17)
  • mobile/src/modules/audio/_components/PlaybackSpeedSetting.tsx
  • mobile/src/modules/audio/_screens.tsx
  • mobile/src/modules/audio/equalizer/components/EQGraph.tsx
  • mobile/src/modules/audio/equalizer/components/EqualizerSettings.tsx
  • mobile/src/modules/audio/equalizer/components/FrequencySlider.tsx
  • mobile/src/modules/audio/equalizer/core/actions.ts
  • mobile/src/modules/audio/equalizer/core/constants.ts
  • mobile/src/modules/audio/equalizer/core/store.ts
  • mobile/src/modules/audio/replayGain/components/PreAmpSlider.tsx
  • mobile/src/modules/audio/replayGain/components/ReplayGainSettings.tsx
  • mobile/src/modules/equalizer/screens/View.tsx
  • mobile/src/modules/scanning/hooks/useSetup.ts
  • mobile/src/navigation/components/MiniPlayer.tsx
  • mobile/src/navigation/routes.tsx
  • mobile/src/navigation/screens/now-playing/sheets/PlaybackOptionsSheet.tsx
  • mobile/src/navigation/screens/settings/ExperimentalSettingsView.tsx
  • mobile/src/resources/icons/Equalizer.tsx
💤 Files with no reviewable changes (3)
  • mobile/src/resources/icons/Equalizer.tsx
  • mobile/src/modules/equalizer/screens/View.tsx
  • mobile/src/navigation/routes.tsx

Comment thread mobile/src/modules/audio/_screens.tsx
@cyanChill cyanChill merged commit 2ddd357 into dev Jun 5, 2026
1 check was pending
@cyanChill cyanChill deleted the refactor/audio-module-movement branch June 5, 2026 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Housekeeping Code maintenance or refactor.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant