[WIP] Mute organization badge and server count #626

kanishk98 · 2019-01-14T09:15:30Z

What's this PR do?
Adds context menu and settings option to mute/unmute badge count of connected organizations.

Any background context you want to provide?
https://github.com/zulip/zulip-electron/issues/623 should provide the required context. Please also have a look at the chat thread referenced.

Screenshots?

You have tested this PR on:

Windows
Linux/Ubuntu
macOS

Adds mutedOrganizations object to settings.json file inside the app's data folder to keep track of muted organizations.

Depending on per-org setting, the app will now hide the badge count of certain servers. Fix unmute/mute confirmation prompt

abhigyank · 2019-01-14T11:22:56Z

app/renderer/js/components/webview.js

@@ -117,6 +117,11 @@ class WebView extends BaseComponent {
 	}

 	getBadgeCount(title) {
+		const mutedOrganizations = ConfigUtil.getConfigItem('mutedOrganizations');
+		const { url } = this.props;
+		if (!!mutedOrganizations[url]) {


Jus out of curiosity, any reason to use !!mutedOrganizations[url] instead of simply mutedOrganizations[url] ?

Not really, just the way I got used to. I'll change this along with the Travis fixes and squash the commits.

abhigyank

The feature works as expected and required. Good work on that.
A couple of suggestions with implementation though -

Instead of storing whether org is muted in configutil, I think i'd be better to store that as a property of the org in domain-util.json.
I feel Muting/Unmuting a realm causing a refresh could be inconvenience to the user. I feel we should be able to do maybe to this without refreshing.
Not sure about the design of the additions in the organisations page. Maybe @akashnimare would weigh in on that.

kanishk98 · 2019-01-14T11:50:43Z

Instead of storing whether org is muted in configutil, I think i'd be better to store that as a property of the org in domain-util.json.

I attempted that initially by adding a notify flag to the serverConf object, but this approach didn't work out for zulipdev.org servers. The flag would get added and then removed from the file. This was before you helped me with the console logs; I'll let you know how this goes.

I feel Muting/Unmuting a realm causing a refresh could be inconvenience to the user. I feel we should be able to do maybe to this without refreshing.

I agree. I'll get to work. :)

kanishk98 · 2019-01-14T13:25:41Z

@abhigyank the latest commit addresses the refresh problem, but behaviour is undesirable in one case - if the user is on the org preferences page and then mutes an org using the context menu, it does not trigger an update of the DOM element in the pane.
My initial intuition is that if we can keep track of where the user is in the app, then we can figure out a way to cause the update in the preferences page if the user mutes the organization using the context menu.

abhigyank · 2019-01-14T14:01:47Z

@kanishk98 the unread count now doesn't change for me in any case. Now since we are not reloading the app, you would need explicitly remove the badge count of the div server-tab-badge of that icon.

if the user is on the org preferences page and then mutes an org using the context menu, it does not trigger an update of the DOM element in the pane.

Not sure how we can tackle this but I think we can make use of IPCRenderers 'forward-message' feature here.

kanishk98 · 2019-01-14T18:03:41Z

Ah, sorry. Should've checked thoroughly.

Not sure how we can tackle this but I think we can make use of IPCRenderers 'forward-message' feature here.

Thanks! I'll check that out.

showell · 2019-01-16T14:21:41Z

Some relevant discussion is here:

https://chat.zulip.org/#narrow/stream/101-design/topic/mute.20organization/near/683125

Individually handles refreshing UI without refreshing the whole app after user mutes/unmutes server for both context menu and org preferences pane. Also improves code readibility by removing !! operations. Mute individual orgs without refreshing entire app Fix Travis errors Fix Travis errors

akashnimare · 2019-01-21T09:24:50Z

@rishig anything else we should be doing here functionality wise?

rishig · 2019-01-24T14:10:16Z

What does the setting currently do?
Also, I think the conclusion was that this should be implemented in the webapp?

rishig · 2019-01-24T14:11:18Z

app/renderer/js/main.js

@@ -530,6 +533,29 @@ class ServerManagerView {
 							}
 						});
 					}
+				},
+				{
+					label: (mutedOrganizations[url] ? 'Unmute' : 'Mute') + ' organization',


Note that stuff like this is not going to work with translations

I'm sorry, I didn't think about that. I'll need to have a look at how translations are currently handled. Will get to it.

kanishk98 · 2019-01-24T21:22:19Z

What does the setting currently do?

The setting hides the number of unread messages in the sidebar and the tray icon for all muted organizations.

Also, I think the conclusion was that this should be implemented in the webapp?

If you see this thread, you'll see that the last few messages indicate that the feature is required in the desktop app too. One of the suggestions from the community (see https://github.com/zulip/zulip-electron/issues/623#issuecomment-454795980) is to extend this functionality to other environments, but this is certainly needed on the desktop. :)

rishig · 2019-01-24T21:41:25Z

ah, I guess we should have been more explicit. Given that it needs to be reflected on all clients, the setting should be stored on the UserProfile object on the server, and controlled by something in the webapp's settings page. That single setting will control how we set unread counts on desktop/mobile/terminal/web.

zulipbot · 2019-04-18T14:11:45Z

Heads up @kanishk98, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

akashnimare · 2019-08-13T20:18:12Z

@kanishk98 any update on this? zulip/zulip#12677 should be helpful I guess. @rishig would it make sense to add an option like the following to mute an org in the desktop app -

rishig · 2019-08-14T06:13:36Z

yeah, for sure; maybe called "None".
Though I imagine the current setting solved most of the problem for most people.

kanishk98 · 2019-08-14T13:21:47Z

I'm confused about the discussion here - given that people can now control their notifications across all clients from the web app and we read notifications from the favicon updates, I thought the conclusion here was to move such settings to the web app and continue as is with the desktop app.

kanishk98 · 2019-08-14T13:22:25Z

If we're planning to also include a similar setting on the desktop app, how does that add more to what's on the web app right now?

akashnimare · 2019-08-14T13:25:04Z

yeah, for sure; maybe called "None".

@kanishk98 if we add this in the webapp then we can re-use it in the desktop app to add muting org feature.

rishig · 2019-08-14T15:37:01Z

I also agree with Kanishk's intuition; I think we likely don't need a separate desktop app feature for this.
The benefit of having settings in the webapp is that those settings will translate to mobile as well.

kanishk98 added 3 commits January 9, 2019 14:50

Add mute button to context menu and org settings

48db7d9

Save muted/unmuted status per org

bc28fec

Adds mutedOrganizations object to settings.json file inside the app's data folder to keep track of muted organizations.

Mute badges and tray icons

5c7542e

Depending on per-org setting, the app will now hide the badge count of certain servers. Fix unmute/mute confirmation prompt

zulipbot added the size: L label Jan 14, 2019

abhigyank reviewed Jan 14, 2019

View reviewed changes

kanishk98 force-pushed the mute-organizations branch from 618d0d9 to 49869d2 Compare January 18, 2019 19:37

zulipbot added size: XL and removed size: L labels Jan 18, 2019

kanishk98 force-pushed the mute-organizations branch from 49869d2 to 5fbd7d8 Compare January 21, 2019 08:25

rishig reviewed Jan 24, 2019

View reviewed changes

kanishk98 changed the title ~~Mute organization badge and server count~~ [WIP] Mute organization badge and server count Apr 5, 2019

zulipbot added the has conflicts label Apr 18, 2019

akashnimare mentioned this pull request Aug 16, 2019

Allow muting organizations #623

Closed

Base automatically changed from master to main January 22, 2021 19:22

Uh oh!

[WIP] Mute organization badge and server count #626

Are you sure you want to change the base?

[WIP] Mute organization badge and server count #626

Uh oh!

Conversation

kanishk98 commented Jan 14, 2019

Uh oh!

abhigyank Jan 14, 2019

Choose a reason for hiding this comment

Uh oh!

kanishk98 Jan 14, 2019

Choose a reason for hiding this comment

Uh oh!

abhigyank left a comment

Choose a reason for hiding this comment

Uh oh!

kanishk98 commented Jan 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kanishk98 commented Jan 14, 2019

Uh oh!

abhigyank commented Jan 14, 2019

Uh oh!

kanishk98 commented Jan 14, 2019

Uh oh!

showell commented Jan 16, 2019

Uh oh!

akashnimare commented Jan 21, 2019

Uh oh!

rishig commented Jan 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rishig Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

kanishk98 Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

kanishk98 commented Jan 24, 2019

Uh oh!

rishig commented Jan 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zulipbot commented Apr 18, 2019

Uh oh!

akashnimare commented Aug 13, 2019

Uh oh!

rishig commented Aug 14, 2019

Uh oh!

kanishk98 commented Aug 14, 2019

Uh oh!

kanishk98 commented Aug 14, 2019

Uh oh!

akashnimare commented Aug 14, 2019

Uh oh!

rishig commented Aug 14, 2019

Uh oh!

Uh oh!

kanishk98 commented Jan 14, 2019 •

edited

Loading

rishig commented Jan 24, 2019 •

edited

Loading

rishig commented Jan 24, 2019 •

edited

Loading