Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset the unread marker on channel change #527

Merged
merged 1 commit into from Jul 26, 2016
Merged

Conversation

maxpoulin64
Copy link
Member

This restores the old behavior of resetting the unread marker on channel change, as that's usually at this point one wants to check for new messages and is also what matches on the server. I feel this is overall more consistent and useful, and also more in line with what other clients do.

@maxpoulin64 maxpoulin64 added Type: Feature Tickets that describe a desired feature or PRs that add them to the project. second review needed labels Jul 24, 2016
@maxpoulin64 maxpoulin64 force-pushed the PR/fix-unread-clear branch 2 times, most recently from 1db58cc to 4d7dfdf Compare July 24, 2016 06:18
This restores the old behavior of resetting the unread marker on channel change, as that's usually at this point one wants to check for new messages and is also what matches on the server. I feel this is overall more consistent and useful, and also more in line with what other clients do.
.find(".chan.active")
.removeClass("active");

lastActiveChan
.find(".unread-marker")
.appendTo(lastActiveChan.find(".messages"));
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this work just fine?

lastActive
            .find(".chan.active")
            .removeClass("active")
            .find(".unread-marker")
            .appendTo(lastActiveChan.find(".messages"));

(I haven't tried, of course 😅)

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because the active window contains all the channels, so we need to find the specific channel. That was my initial solution before I ended up with a million unread markers in ever channels.

@astorije astorije self-assigned this Jul 26, 2016
@astorije
Copy link
Member

@maxpoulin64, I get your fix, but by any chance did you find out where this bug was introduced? I mean, at some point it was working, and then it was not anymore. I'd like to find the exact piece of code that broke it, for completeness.

@astorije
Copy link
Member

So it seems to me that it's this and/or that but I can't pinpoint the exact change that triggered the bug (I didn't look too closely though). Can you?

@maxpoulin64
Copy link
Member Author

That would be 26bf948

And wow, I really pushed that fix? Damn, I really was tired that week. I thought it was that way by design at first with the new style.

Well, either way, fixed :S

@astorije
Copy link
Member

👍 and merging.

@astorije astorije merged commit 61ea831 into master Jul 26, 2016
@astorije astorije deleted the PR/fix-unread-clear branch July 26, 2016 14:28
@astorije astorije modified the milestone: 2.0.0 Aug 6, 2016
@astorije astorije added Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. and removed Type: Feature Tickets that describe a desired feature or PRs that add them to the project. labels Aug 6, 2016
matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
Reset the unread marker on channel change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants