Page MenuHomePhabricator

Make "No Notifications" setting less broad, and fix a bug with default display behavior
ClosedPublic

Authored by epriestley on Sep 13 2017, 9:59 PM.
Tags
None
Referenced Files
F15554115: D18600.id44660.diff
Mon, Apr 28, 5:34 AM
F15552378: D18600.diff
Sun, Apr 27, 8:39 PM
F15546989: D18600.diff
Sat, Apr 26, 5:39 PM
F15546675: D18600.id44660.diff
Sat, Apr 26, 4:28 PM
F15543789: D18600.id44661.diff
Sat, Apr 26, 1:05 AM
F15531901: D18600.id44659.diff
Wed, Apr 23, 1:43 PM
F15473706: D18600.id44660.diff
Apr 6 2025, 12:50 AM
F15455148: D18600.id44659.diff
Mar 29 2025, 10:33 PM
Subscribers
None

Details

Summary

Fixes T12979. In D18457, we added a "No Notifications" setting to let users disable the blue and yellow pop-up notifications that alert you when an object has been updated, since some users found them distracting.

However, the change made "do nothing" the default, so all other JX.Notification callsites -- which never pass a preference -- were effectively turned off no matter what your setting was set to. This includes the "Read-Only" mode warning (grey), the "High Security" mode warning (purple), the "timezone" warning, and a few others.

Tweak things a little bit so the setting applies to ONLY blue and yellow ("object you're following was updated" / "this object was updated") notifications, not other types of popup notifications.

Test Plan
  • With notifications on in settings, got blue notifications and "Read-only".
  • With notifications off in settings, got "Read-only" but no blue notifications.

Diff Detail

Repository
rP Phabricator
Branch
cluster1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 18439
Build 24829: Run Core Tests
Build 24828: arc lint + arc unit

Event Timeline

src/applications/notification/builder/PhabricatorNotificationBuilder.php
174–175

Er, I think these are probably not wrong exactly but the old version was clearer, I'll use something more similar.

  • Use slightly clearer construction in NotificationBuilder.
This revision is now accepted and ready to land.Sep 13 2017, 10:09 PM