mattermost / desktop

Mattermost Desktop application for Windows, Mac and Linux
Apache License 2.0
2.01k stars 824 forks source link

Direct messages cause notification icons to appear on each team, which don't clear until clicked on each team #160

Closed leipert closed 7 years ago

leipert commented 8 years ago

Hey @yuya-oc, we currently have the problem, that we have two teams with the same users. So basically the unread count is increased in both teams, for every direct message.

Now that I found out, that you are getting the unread count by parsing html, would you be open to a PR with more direct approach and retrieving the unread count directly with the API?

Anyway, thank you for your work on the client. I would look forward to a possible collaboration.

/cc @crspeller @it33

yuya-oc commented 8 years ago

Sure, currently the client parses html (btw, the part you linked is for 'unread channels'. The mention count is gotten from sidebar and mention-highlight). Of course PR is welcome. However, probably that makes breaking change, so it might take long time to review. Then, unfortunately I don't know well about the API, so now I can't tell enough advise to implement.

jasonblais commented 8 years ago

Thanks @leipert for the feedback. Very much appreciated,

We have a related issue that was recently opened in the /platform repo: https://github.com/mattermost/platform/issues/3529. The team is open for suggestions on the expected behaviour when handling direct message lists across multiple teams. Feel free to add your thoughts by participating in the discussion.

jasonblais commented 8 years ago

Adding repro steps from @jfpreusse-bhvr:

Summary

When receiving a direct message, a red notification icon is added to all tabs and can only be cleared by opening the message on each team.

Steps to reproduce

This is for the Windows Desktop Application version 1.3.0.

Expected behavior

The direct message notification icon should be displayed on one team only (e.g. the main team on the server, or make this a user preference), or at least be cleared for all tabs when read on one of the teams.

Observed behavior

The direct message notification icon is displayed on all the teams. The user must open the message in each team for all notification icons to be cleared.

drmikecrowe commented 8 years ago

So, in taking a cursory look at this, is there a reason to map the notifications to all teams rather than just the first team? i.e.

src/browser/index.jsx:

var views = this.props.teams.map(function(team, index) {
      if (index > 0) return;        // only send to the first team????
      var handleUnreadCountChange = function(unreadCount, mentionCount, isUnread, isMentioned) {
        thisObj.handleUnreadCountChange(index, unreadCount, mentionCount, isUnread, isMentioned);
      };
      var handleNotificationClick = function() {
        thisObj.handleSelect(index);
      }
      var id = 'mattermostView' + index;
      var is_active = thisObj.state.key === index;
      return (<MattermostView key={ id } id={ id } style={ thisObj.visibleStyle(is_active) } src={ team.url } name={ team.name } onUnreadCountChange={ handleUnreadCountChange }
                onNotificationClick={ handleNotificationClick } ref={ id } active={ is_active } />)
    });
yuya-oc commented 8 years ago

For earlier mattermost servers, it was necessary because users were separately managed by each team even if they exist in the same server.

Then, the code you pointed is the wrong part. views is actually an array of <webview> and they just report their unread counting. The appearance of counting is managed by <Tabbar> component. Sure, the simple mapping causes this problem, so need to organize the counting.

jasonblais commented 8 years ago

Mattermost has a feature in review where teams belonging to the same server would appear on the sidebar, left of the channels.

image

I think once the sidebar for teams gets implemented, we can much more easily streamline how notifications across teams work. Hence, adding a Pending label to this issue for now

jasonblais commented 7 years ago

Adding server label as this is blocked by the teams sidebar on the server side.

jfperusse-bhvr commented 7 years ago

Adding my suggestion from the closed issue 3529.

Here's how I think this should work. This is one of the most annoying issue for us since we're all part of multiple teams.

Suppose that:

What should happen:

To me this makes a lot of sense since I generally talk to another user from the team which makes the most sense for the topic I want to discuss.

jasonblais commented 7 years ago

@leipert @drmikecrowe @jfperusse-bhvr This issue should be resolved in Mattermost 3.6 with the new multi-deployment support through team sidebar. The release will be on January 16th.

With the new release, all of your teams will be visible on the left-hand sidebar as an additional team, and therefore you no longer need a separate tab for each team.

To update your team management tabs, simply go to File > Settings or click Ctrl+Comma on your keyboard to visit the App Settings page.