Clean up semantic-domain-clicking behavior

[imnasnainaec]

This pr fixes two subpar behaviors in the find-related-words WebView.

1. In the main list, clicking a semantic domain in a list item updated the list, but had the undesirable side-effect of opening that entry. Fixes with e.stopPropagation().
2. In the drawer, clicking a semantic domain updated the main list, but didn't close the drawer. Fixed with a prop function refactor.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors back-to-list button interaction patterns across multiple dictionary display components. The BackToListButton callback signature simplifies to accept no arguments with the dictionaryEntry prop removed. DictionaryEntryDisplay adds semantic domain click handling, DictionaryList updates its callback behavior, and DomainsDisplay adds event propagation guards.

Changes

Cohort / File(s)	Change Summary
Back-to-list Button Refactoring platform.bible-extension/src/components/back-to-list-button.tsx	Simplifies callback signature from (option: ListboxOption) => void to () => void; removes dictionaryEntry prop; adds localizedStrings prop; wraps button with DrawerClose when in drawer mode
Dictionary Entry Display platform.bible-extension/src/components/dictionary-entry-display.tsx	Updates handleBackToListButton callback signature to accept no arguments; adds new onClickSemanticDomain prop; introduces internal onClickDomain wrapper for domain click handling with drawer-aware behavior; removes dictionaryEntry prop from BackToListButton usage
Dictionary List platform.bible-extension/src/components/dictionary-list.tsx	Introduces clearSelectedEntry function; replaces handleOptionSelect callback with clearSelectedEntry for back-to-list behavior in both drawer and wide-screen paths
Domains Display platform.bible-extension/src/components/domains-display.tsx	Adds event propagation guards (stopPropagation) to domain pill click handler; guards callback invocation with explicit null check instead of optional chaining

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Callback chain in DictionaryEntryDisplay: Verify the onClickSemanticDomain → onClickDomain → handleBackToListButton flow works correctly in drawer and non-drawer contexts
• Event propagation in DomainsDisplay: Confirm stopPropagation doesn't interfere with intended parent handlers or drawer close behavior
• Integration between components: Ensure BackToListButton simplification doesn't break existing drawer close functionality across all usage sites

Possibly related PRs

• sillsdev/languageforge-lexbox#2004: Modifies the same components and prop signatures (BackToListButton, DictionaryEntryDisplay, DictionaryList, DomainsDisplay) with directly related code-level changes

Suggested labels

📙 Platform.Bible, 🟨 Medium

Suggested reviewers

• jasonleenaylor

Poem

🐰 Buttons back and forth we go,
Callbacks simple, clean, and so
Domains clicked with guards in place,
Drawers close with newfound grace,
Back-to-list, a lighter way—
Refactored smooth for today! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to evaluate whether a description exists and relates to the changeset.	Add a description explaining the motivation for these changes, what was cleaned up, and any behavioral implications for reviewers.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Clean up semantic-domain-clicking behavior' accurately describes the main change: refactoring the semantic domain click handling across multiple components to simplify callbacks and remove unnecessary props.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pb-ext-domain-click-not-word-click

---

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]

Actionable comments posted: 0

🧹 Nitpick comments (1)

platform.bible-extension/src/components/domains-display.tsx (1)

41-46: LGTM with a minor observation.

The addition of e.stopPropagation() correctly prevents the click from bubbling to parent handlers. However, the explicit if (onClickDomain) check on line 42 is redundant since the button is already disabled={!onClickDomain} on line 39, preventing clicks when the handler is undefined.

Optional simplification:

-          onClick={(e) => {
-            if (onClickDomain) {
-              e.stopPropagation();
-              onClickDomain(domain);
-            }
-          }}
+          onClick={(e) => {
+            e.stopPropagation();
+            onClickDomain?.(domain);
+          }}

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6b416be and 0cc7f19.

📒 Files selected for processing (4)

• platform.bible-extension/src/components/back-to-list-button.tsx (2 hunks)
• platform.bible-extension/src/components/dictionary-entry-display.tsx (4 hunks)
• platform.bible-extension/src/components/dictionary-list.tsx (3 hunks)
• platform.bible-extension/src/components/domains-display.tsx (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: imnasnainaec
Repo: sillsdev/languageforge-lexbox PR: 1867
File: platform.bible-extension/src/main.ts:239-246
Timestamp: 2025-07-31T19:10:41.178Z
Learning: In the sillsdev/languageforge-lexbox repository, user imnasnainaec prefers to defer code improvements when there are related TODO comments indicating planned refactoring work, choosing to bundle related changes together rather than making incremental improvements that would need to be modified again during the larger refactoring.

📚 Learning: 2025-07-31T16:00:49.635Z

Learnt from: imnasnainaec
Repo: sillsdev/languageforge-lexbox PR: 1867
File: platform.bible-extension/src/types/fw-lite-extension.d.ts:4-22
Timestamp: 2025-07-31T16:00:49.635Z
Learning: In the sillsdev/languageforge-lexbox repository, the platform.bible-extension is intentionally tightly coupled with the frontend's dotnet-types. The relative imports from `../../../frontend/viewer/src/lib/dotnet-types/index.js` in the extension's type declarations are by design, not a maintainability issue that needs to be addressed.

Applied to files:

• platform.bible-extension/src/components/dictionary-entry-display.tsx
• platform.bible-extension/src/components/back-to-list-button.tsx

📚 Learning: 2025-12-05T10:28:57.551Z

Learnt from: CR
Repo: sillsdev/languageforge-lexbox PR: 0
File: frontend/viewer/AGENTS.md:0-0
Timestamp: 2025-12-05T10:28:57.551Z
Learning: Applies to frontend/viewer/**/*.{svelte,ts,tsx,js,jsx} : Use Lingui with svelte-i18n-lingui for internationalization (i18n)

Applied to files:

• platform.bible-extension/src/components/dictionary-entry-display.tsx

🧬 Code graph analysis (1)

platform.bible-extension/src/components/dictionary-entry-display.tsx (1)

platform.bible-extension/src/types/fw-lite-extension.d.ts (1)

• ISemanticDomain (14-14)

🔇 Additional comments (7)

platform.bible-extension/src/components/dictionary-list.tsx (2)

56-58: LGTM!

The new clearSelectedEntry helper provides clearer semantics for the back-to-list action. Previously, handleOptionSelect toggled the selection state, which was less intuitive. The explicit clear operation better matches user expectations when returning to the list.

---

130-130: LGTM!

Consistently using clearSelectedEntry for the back-to-list behavior in both wide-screen and drawer modes ensures predictable UX.

Also applies to: 151-151

platform.bible-extension/src/components/back-to-list-button.tsx (2)

10-10: LGTM!

Simplifying the callback signature to () => void is appropriate since the back-to-list action doesn't need dictionary entry data. This reduces coupling between components.

---

32-42: LGTM!

The conditional DrawerClose wrapper cleanly handles the drawer-specific close behavior, while the simplified onClick handler reflects the streamlined callback signature.

platform.bible-extension/src/components/dictionary-entry-display.tsx (3)

52-60: LGTM!

The onClickDomain wrapper elegantly coordinates semantic domain clicks with drawer closure. When a user clicks a domain in drawer mode, they get both the domain navigation and a smooth return to the list view.

---

25-25: LGTM!

The prop signature changes align well with the overall refactoring:

• Simplified handleBackToListButton callback removes unnecessary coupling
• New onClickSemanticDomain prop enables the domain-click coordination pattern

Also applies to: 29-29

---

115-115: LGTM!

Passing the onClickDomain wrapper (instead of onClickSemanticDomain directly) ensures that domain clicks properly trigger drawer closure in drawer mode.