Skip to content

fix(desktop): ensure notification click switches to correct tab#936

Merged
Kitenite merged 1 commit intosuperset-sh:mainfrom
kkjcheng:clicking-on-notification-doesnt-bring-you-to-right
Jan 25, 2026
Merged

fix(desktop): ensure notification click switches to correct tab#936
Kitenite merged 1 commit intosuperset-sh:mainfrom
kkjcheng:clicking-on-notification-doesnt-bring-you-to-right

Conversation

@kkjcheng
Copy link
Copy Markdown
Contributor

@kkjcheng kkjcheng commented Jan 24, 2026

Summary

  • Fix notification click not switching to the correct tab when already on the right workspace
  • Add fallback to use event's tabId directly when pane-based resolution fails
  • Add debug logging to help diagnose future tab activation issues

Test plan

  • Click a notification while on the correct workspace but different tab
  • Verify the app switches to the tab that triggered the notification
  • Verify pane focus also works correctly

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced reliability of tab and pane focus operations by implementing fallback mechanisms to handle missing or unresolved identifiers, ensuring graceful behavior in edge cases where expected data may not be available.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 24, 2026

📝 Walkthrough

Walkthrough

This change hardens the FOCUS_TAB event flow in a tab listener by introducing fallback mechanisms for resolving tabId and paneId, ensuring the handler gracefully degrades when explicit IDs are unavailable by falling back to event data.

Changes

Cohort / File(s) Summary
Tab focus event resilience
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts
Added fallback logic to resolve tabId and paneId from event data when target IDs are missing; refactored FOCUS_TAB handler to check for resolved pane existence before calling setFocusedPane and use fallback tabId throughout the flow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • Kitenite

Poem

🐰 A rabbit hops through tab events with care,
When IDs go missing, fallbacks are there!
From target to data, a graceful retreat,
Making focus resilience perfectly neat. 🎯✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers the problem, solution approach, and test plan, but deviates from the repository template by using a custom structure instead of the required sections like Type of Change, Testing, and Related Issues. Restructure the description to follow the template: add Type of Change section, expand Testing section with detailed steps, and include Related Issues section with any GitHub issue links.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main fix: ensuring notification clicks switch to the correct tab, which is the primary objective of this changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

When clicking a notification while on the correct workspace but wrong tab,
the tab switch wasn't happening. The issue was that resolveNotificationTarget
might return undefined for tabId if the pane state was stale.

Added fallback to use event's tabId directly when pane-based resolution fails,
plus debug logging to help diagnose future issues.
@kkjcheng kkjcheng force-pushed the clicking-on-notification-doesnt-bring-you-to-right branch from 0e24ed0 to 8110d80 Compare January 24, 2026 22:18
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @kkjcheng !

@Kitenite Kitenite merged commit de8bb50 into superset-sh:main Jan 25, 2026
5 checks passed
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