fix(region-selector): prevent empty region reporting during sync

[0xSysR3ll]

Description

This PR fixes an issue where reopening the filter panel could clear the selected streaming service filters.

The problem was caused by RegionSelector emitting onChange('watchRegion', '') during its initial state sync, before the region value from the parent was applied. WatchProviderSelector interpreted this as a real change and cleared the selected providers, which also removed the filter from the URL.

• Fixes Streaming service filter reset when panel is reopened #2622

How Has This Been Tested?

Screenshots / Logs (if applicable)

Checklist:

• I have read and followed the contribution guidelines.
• Disclosed any use of AI (see our policy)
• I have updated the documentation accordingly.
• All new and existing tests passed.
• Successful build pnpm build
• Translation keys pnpm i18n:extract
• Database migration (if required)

Summary by CodeRabbit

• Bug Fixes
  • Region selector now handles selections more reliably and consistently notifies the parent of changes, ensuring external value updates are reflected correctly.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0d762dda-cc85-4a5a-bfd5-0d0698435f5f

📥 Commits

Reviewing files that changed from the base of the PR and between 1013f3c and 8aeeae5.

📒 Files selected for processing (1)

• src/components/RegionSelector/index.tsx

---

📝 Walkthrough

Walkthrough

Replaced an effect that auto-invoked onChange based on selectedRegion with a dedicated handler handleRegionSelect. The Listbox onChange now calls this handler, which updates internal state and calls onChange(name, region?.iso_3166_1 ?? ''). The prior effect that auto-called onChange was removed.

Changes

Cohort / File(s)	Summary
RegionSelector Logic src/components/RegionSelector/index.tsx	Removed effect that auto-invoked onChange for selectedRegion; added handleRegionSelect to centralize selection logic. Listbox onChange now delegates to handleRegionSelect, which updates selectedRegion and calls onChange(name, region?.iso_3166_1 ?? '').

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

bug

Suggested reviewers

• fallenbagel
• M0NsTeRRR

Poem

🐰 I hopped in, changed a tiny thread,
Now regions speak when selections are said.
No stray effect calling with eerie might,
Just one clear handler — tidy and right. ✨

🚥 Pre-merge checks | ✅ 4

✅ 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 describes the main change: preventing empty region reporting during sync in the RegionSelector component, which directly addresses the root cause of the linked issue.
Linked Issues check	✅ Passed	The changes directly address issue #2622 by modifying RegionSelector to emit onChange only on user selection, preventing empty value emission during initial sync that was clearing streaming service filters.
Out of Scope Changes check	✅ Passed	All changes are scoped to RegionSelector component and directly address the root cause of the linked issue without introducing unrelated modifications.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/components/RegionSelector/index.tsx (1)

86-94: ⚠️ Potential issue | 🟠 Major

Prevent emitting stale region on external value prop updates.

When the parent component updates the value prop, the sync effect queues a setSelectedRegion state update while the emitter effect runs synchronously in the same effect flush. This causes the emitter to call onChange with the previous selectedRegion before the sync completes. In WatchProviderSelector, this stale emission can unexpectedly trigger setActiveProvider([]).

Proposed fix (emit only on actual selected-region transitions)

-import { useEffect, useMemo, useState } from 'react';
+import { useEffect, useMemo, useRef, useState } from 'react';

+  const lastEmittedRegionRef = useRef<string | null>(null);
+
   useEffect(() => {
     if (onChange && regions) {
-      if (selectedRegion) {
-        onChange(name, selectedRegion.iso_3166_1);
-      } else if (!value) {
+      const nextRegion = selectedRegion?.iso_3166_1 ?? '';
+      if (lastEmittedRegionRef.current === nextRegion) {
+        return;
+      }
+      lastEmittedRegionRef.current = nextRegion;
+
+      if (nextRegion) {
+        onChange(name, nextRegion);
+      } else if (!value) {
         onChange(name, '');
       }
     }
   }, [onChange, selectedRegion, name, regions, value]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/RegionSelector/index.tsx` around lines 86 - 94, The emitter
effect is firing with a stale selectedRegion when the external value prop
triggers a state update in the same render flush; to fix, change the useEffect
(the effect that currently depends on [onChange, selectedRegion, name, regions,
value]) so it only emits when selectedRegion actually transitions: add a
prevSelectedRegionRef (or similar) to store the previous
selectedRegion.iso_3166_1, compare prev vs current inside the effect, and only
call onChange(name, selectedRegion.iso_3166_1) when they differ; after emitting
update the ref. Ensure you still handle the else case for clearing
(onChange(name, '') ) only when the previous selectedRegion was non-empty and
the new one is empty to avoid stale emissions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/components/RegionSelector/index.tsx`:
- Around line 86-94: The emitter effect is firing with a stale selectedRegion
when the external value prop triggers a state update in the same render flush;
to fix, change the useEffect (the effect that currently depends on [onChange,
selectedRegion, name, regions, value]) so it only emits when selectedRegion
actually transitions: add a prevSelectedRegionRef (or similar) to store the
previous selectedRegion.iso_3166_1, compare prev vs current inside the effect,
and only call onChange(name, selectedRegion.iso_3166_1) when they differ; after
emitting update the ref. Ensure you still handle the else case for clearing
(onChange(name, '') ) only when the previous selectedRegion was non-empty and
the new one is empty to avoid stale emissions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 38de2905-d9d2-4394-8e35-119b739d9b3e

📥 Commits

Reviewing files that changed from the base of the PR and between 1548948 and 1013f3c.

📒 Files selected for processing (1)

• src/components/RegionSelector/index.tsx

[Copilot]

Pull request overview

This PR addresses an issue in the discover filter UI where reopening the filter panel could clear selected streaming service providers due to RegionSelector emitting an empty region during its initial sync.

Changes:

• Prevent RegionSelector from emitting an empty region during initial state sync when a non-empty value is provided.
• Update the useEffect dependency list to include value so the sync logic reacts to external prop changes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.