Skip to content
This repository has been archived by the owner on May 4, 2022. It is now read-only.

fix: Obscure notifications bell when on full view #571

Closed
wants to merge 1 commit into from

Conversation

thevoiceofzeke
Copy link
Contributor

@thevoiceofzeke thevoiceofzeke commented Oct 11, 2017

MUMMNG-2880: "As a user dismissing/un-dismissing notifications on the notification page, I would like the bell icon to update to reflect my new unread count, so that consistency is maintained."

In this PR:

  • Hide notification bell (and mobile menu count) when user is viewing the full notifications page

Why?

Because the notifications-bell (in its many forms) and view__notifications templates both use NotificationsController to establish and manipulate scope, you would think the scope would be shared among all the things that use that controller. However, this is not the case. The scope of the full view is isolated from the scope(s) of the various instances of the directive. This means that communication between them has to happen on the $rootScope level (barring yet another involved refactor of the notifications/announcements architecture). A truly syncopated experience without a prohibitive amount of work would depend upon $rootScope.$broadcast or manipulation of a $rootScope variable every time a notification is dismissed or restored, which means messily updating scope in up to 5 places every time. Additionally, the functionality of the full notifications view and the bell directive are nearly identical (a.k.a. redundant).

For the above reasons, it's simpler and arguably better practice to just hide the bell when users are already on the notifications page.

Screenshots

screen shot 2017-10-11 at 10 34 32 am
screen shot 2017-10-11 at 10 46 44 am


Contributor License Agreement adherence:

Copy link
Contributor

@vertein vertein left a comment

Choose a reason for hiding this comment

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

Interesting...

I would not have thought of this.

@apetro
Copy link
Contributor

apetro commented Oct 11, 2017

I suspect displaying and mutating the bell when the user takes an action affecting active notification count elsewhere would help the user to relate the bell to the action, would help the user understand these are different views on the same thing. So arguably that might be a better experience. But this solution seems nonetheless totally acceptable and reduces some aspects of the complexity to be maintained, so, +1.

@thevoiceofzeke
Copy link
Contributor Author

Closing this pull request because it's a duplicate (and slightly behind its twin). See #572

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants