Open message in stream clickable from notifications #1996

Closed
sup95 opened this Issue Oct 13, 2016 · 8 comments

Projects

None yet

2 participants

@sup95
Contributor
sup95 commented Oct 13, 2016

When online on zulip, notifications pop up towards the top right corner. ( I do not mean the desktop notifications, I mean the notifications within zulip.) It would be nice if clicking on the notification indicating the message takes the user to the stream and topic where that particular message is typed. Currently this notification box has only a close tab. I would like that and if there are more people who would find this convenient, perhaps this could be something to implement? Thanks. :)

@timabbott
Member

Yeah, we should definitely change that, and I suspect it wouldn't be too hard; the code for our notifications is all in static/js/notifications.js and I suspect we just need to change the handle for what happens when one clicks on them.

@sup95
Contributor
sup95 commented Oct 16, 2016 edited

I was going through the code in notifications.js and click_handlers.js and came up with the following fix.
Does this look right? (Adding the following code to in_browser_notify function in notifications.js)

$(".top-right").on("click", ".narrows_by_recipient", function (e) {
        if (e.metaKey || e.ctrlKey) {
            return;
        }
        e.preventDefault();
        var row_id = get_row_id_for_narrowing(this);
        narrow.by_recipient(row_id, {trigger: 'message header'});
    });

I am new to fixing front end bugs, being more of a python person. But I really wanted to solve this bug. Hence, running it by you before making a PR.
Thank you.

@timabbott
Member

Yes, I think something like that should work.

@sup95
Contributor
sup95 commented Oct 17, 2016

Cool. I'll submit a PR soon. Also, how can I test this on my local system? Since it requires me to receive a message from someone else? Thank you.

@timabbott
Member

To test on a local system, you'll want to open two browsers (say, Chrome and Firefox), and login to different users in the two browsers on your development environment. Then, send a message from the user in one browser to the user in the other.

@sup95 sup95 added a commit to sup95/zulip that referenced this issue Oct 19, 2016
@sup95 sup95 Pop-up notification leads to message. Fix #1996. 9d62b43
@sup95
Contributor
sup95 commented Oct 25, 2016

@timabbott Hi! Could you please have a look at the patch I submitted and let me know if there are any changes to be made? Thank you! :)

@timabbott
Member

Done! Thanks for working on this; I posted a comment on how you could improve the behavior in this code.

@timabbott
Member

@sup95 I just wanted to check in on whether you're planning to finish that implementation?

@timabbott timabbott added this to the Zulip roadmap milestone Nov 18, 2016
@showell showell added a commit that closed this issue Dec 24, 2016
@KingxBanana @showell KingxBanana + showell notifications.js: Make in-browser notifications clickable
You can now click the notifications you get in your browser and go
to that stream/topic/private message using jQuery's .on() method.

Fixes: #1996.
5f77ce1
@showell showell closed this in 5f77ce1 Dec 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment