Skip to content

Conversation

@synoet
Copy link
Contributor

@synoet synoet commented Nov 27, 2025

Adds support for clicking on a browser / platform notification to open the notification. Centralizes some of the sprawling logic for doing this within the app.

@linear
Copy link

linear bot commented Nov 27, 2025

M-5269 browser notification click should go to notification

Clicking on a browser notification should take you directly to the notification in the channel or in the email

@seanaye seanaye force-pushed the synoet/m-5269-browser-notification-click-should-go-to-notification branch from 6203112 to fb306f1 Compare November 27, 2025 22:14
@seanaye seanaye force-pushed the main branch 2 times, most recently from 5e5b6ba to 42fd90f Compare November 28, 2025 17:13
@seanaye seanaye force-pushed the synoet/m-5269-browser-notification-click-should-go-to-notification branch from fb306f1 to 06a750d Compare November 28, 2025 17:13
@seanaye seanaye force-pushed the synoet/m-5269-browser-notification-click-should-go-to-notification branch from 06a750d to b37c9be Compare November 28, 2025 17:33
…browser-notification-click-should-go-to-notification
@synoet synoet requested review from nickisnoble and sedson November 30, 2025 22:48
@synoet synoet marked this pull request as ready for review November 30, 2025 22:48
@synoet synoet requested a review from a team as a code owner November 30, 2025 22:48
Copy link
Contributor

@sedson sedson left a comment

Choose a reason for hiding this comment

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

lgtm with question

replaceOrInsertSplit({ type: 'email', id: targetId });
break;
handle?.goToLocationFromParams({
messageId,
Copy link
Contributor

Choose a reason for hiding this comment

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

is this for both email and channel? I don't think those are the right param names for channels - idk about email. it's ugly but we should be importing the URL_PARAM constants from the blocks here i think?

n.eventItemId
)
)
.with(
Copy link
Contributor

Choose a reason for hiding this comment

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

damn ok new lib

@synoet synoet merged commit 44ae250 into main Dec 1, 2025
18 checks passed
@synoet synoet deleted the synoet/m-5269-browser-notification-click-should-go-to-notification branch December 1, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants