Skip to content

Dropdown Notification for Community #3337

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

Closed
ajefts opened this issue Sep 26, 2019 · 28 comments
Closed

Dropdown Notification for Community #3337

ajefts opened this issue Sep 26, 2019 · 28 comments

Comments

@ajefts
Copy link
Member

ajefts commented Sep 26, 2019

Overview

Enable notifications for community members. These will be similar to what we have for Connect users. The purpose is to keep members updated and aware of activities they are required to complete as well pushing info to them that might be of interest.

References

@ajefts ajefts added the topgear label Sep 26, 2019
@ajefts
Copy link
Member Author

ajefts commented Sep 26, 2019

@Oanh-and-only-Oanh Oanh-and-only-Oanh changed the title Notification Center for Community Dropdown Notification for Community Jan 30, 2020
@luizrrodrigues
Copy link
Collaborator

Notifications Popup is done and already merged with community-app notifications branch.
Challenge: http://www.topcoder.com/challenges/30114259
Fixes (F2F): http://www.topcoder.com/challenges/30115261

Notifications Listings challenge is LIVE (submission deadline: 02.22.2020)
http://www.topcoder.com/challenges/30116087

@SathyaJayabal
Copy link
Collaborator

  1. Can you provide the access to
    https://docs.google.com/spreadsheets/d/1AMPSBj_wUdMM-hs9oaVRubSkhAnk3bKTjAxcDn8rx60/edit?ts=5cc888f3#gid=967851284

  2. What are the states of the notifications in the pop-up?
    There is "un seen" with a pink dot. There is "un read" with the grey dot. There seems to be another state "read", with no dot. What is the use of this state ? In connect( and in general) when a notification is read it is removed from the list. Will the notifications be removed when the related contest is completed or when the user closes the respective completed challenge notification?

Screenshot 2020-02-20 at 9 03 21 AM

  1. Should the "Mark All as Read" should be disabled once all the notifications are read? Same with the tick in the mobile view.

Screenshot 2020-02-20 at 8 47 53 AM

  1. Mobile view: Will the notification icon and the number of notifications be always in pink or only if there are unread notifications? If not what is the colour for the other two states?

Screenshot 2020-02-20 at 9 28 36 AM

  1. The same set of notifications are displayed when we reload the page, even though we have closed/read some of the notifications. Why are user actions not saved? Is it for testing purposes?

cc @luizrrodrigues cc @Oanh-and-only-Oanh

@SathyaJayabal
Copy link
Collaborator

UI Issues:

  1. The space between the notification icon and the pink dot seems more than the design

Screenshot 2020-02-20 at 8 41 54 AM

Design:

Screenshot 2020-02-20 at 9 52 23 AM

  1. The hover state background circle is not centered.

Screenshot 2020-02-20 at 8 42 01 AM

Screenshot 2020-02-20 at 8 46 22 AM

  1. The notifications title is not aligned correctly in mobile view

Screenshot 2020-02-20 at 9 54 35 AM

cc @luizrrodrigues

@SathyaJayabal
Copy link
Collaborator

@luizrrodrigues , not able to test the empty state as I am not able to remove the notifications.

@SathyaJayabal SathyaJayabal added Need clarification Need clarification to proceed fixing the issue further QA Fail QA verification on Dev has failed. Assignee to redo the fix. Test Env Environment labels Feb 20, 2020
@luizrrodrigues
Copy link
Collaborator

@luizrrodrigues , not able to test the empty state as I am not able to remove the notifications.

Let cover all issues in populated state first, then I'll clear the notifications to we test empty state.

@luizrrodrigues
Copy link
Collaborator

  1. @SathyaJayabal Please check challenge spec:
    https://www.topcoder.com/challenges/30114259

  2. @Oanh-and-only-Oanh Please confirm this one.

  3. @Oanh-and-only-Oanh Please confirm this one.

  4. @SathyaJayabal Yes, currently we're using mock data, so always when refresh will load previous data.


UI issues, I'll fix all.

@SathyaJayabal
Copy link
Collaborator

SathyaJayabal commented Feb 20, 2020

From the challenge spec:
Mark All as Read - will remove all notifications
All the notifications that have a pink dot are new notifications that have not been seen before (not seen means a user didn't open the popup)
All notifications who have a gray dot are notifications that have been see (meaning the user opened the popup before) but have not been read (as in the user didn't click them)

@luizrrodrigues, Marking as read does not remove the notifications at present. It just removes the grey dot.

@SathyaJayabal
Copy link
Collaborator

SathyaJayabal commented Feb 20, 2020

Completed challenges: why do they have a close icon to dismiss the notification ? Why cant they be dismissed the usual way, by marking them as read ? What is the need for two different ways to dismiss the notifications ?
cc @luizrrodrigues @Oanh-and-only-Oanh

@Oanh-and-only-Oanh
Copy link

Oanh-and-only-Oanh commented Feb 20, 2020

@SathyaJayabal @luizrrodrigues

  1. Should the "Mark All as Read" should be disabled once all the notifications are read? Same with the tick in the mobile view. Yes, check mark should be disabled once all the notifications are read (mobile and desktop).

  2. Mobile view: Will the notification icon and the number of notifications be always in pink or only if there are unread notifications? If not what is the colour for the other two states?
    image
    Icon should be "disabled or greyed out" if all notifications have been read. If there are no new notifications, or all notifications have been read, there shouldn't be a number next to the icon. @Dara-K can you weigh in for design perspective?

@SathyaJayabal
Copy link
Collaborator

@luizrrodrigues , one more UI issue. The handle should be bold like in the design.
Screenshot 2020-02-20 at 10 39 28 PM
@Oanh-and-only-Oanh @Dara-K , should the handle be a link?

@Dara-K
Copy link

Dara-K commented Feb 20, 2020

@Oanh-and-only-Oanh @SathyaJayabal Here's the design for that screen when the notifications are seen: https://marvelapp.com/dcc2h8h/screen/66552240

@Dara-K
Copy link

Dara-K commented Feb 20, 2020

I don't think we want the handle to be a link - that would take the members to the user profile, which doesn't bring any value here, but from a UI perspective if we make the handle link, we have to make it blue and underlined to pass accessibility test, which would just distract the user from the notification itself.

@Oanh-and-only-Oanh what are your thoughts on this?

@SathyaJayabal
Copy link
Collaborator

@Dara-K , I agree, the handle as a link does not add any value and will distract from the actual notification. Forget that I asked :)

@SathyaJayabal
Copy link
Collaborator

@Oanh-and-only-Oanh , @Dara-K , are the notifications themselves clickable like in connect? Do they take the user to the appropriate page (eg: to the challenge details - checkpoints tab when the checkpoint review is ready etc) ?

@luizrrodrigues
Copy link
Collaborator

@SathyaJayabal

Mark All as Read - will remove all notifications

This was a spec typo, Mark All as Read will just remove gray/pink marks.


@Dara-K Thanks for clarifications, just to confirm:

a) In mobile, the notifications count is only to unseen notifications?
b) If only have unread notifications, need to show count too?
c) If we have both, unread and unseen notifications:

  • Show the bell icon in red color;
  • Show counter in red color;
  • The counter will be unseen or unread count?

@SathyaJayabal
Copy link
Collaborator

@luizrrodrigues , so the notifications will be removed when the challenge is completed?

@luizrrodrigues
Copy link
Collaborator

@SathyaJayabal when challenge is completed will move to Completed Challenges at bottom:
image

In this section we have X button, click in this button will remove notification from panel.

@Dara-K
Copy link

Dara-K commented Feb 23, 2020

@SathyaJayabal Yes, it would be great if we could make the notifications clickable as in Connect to take the user to the related page.

@luizrrodrigues
a) Right
b) No
c)

  • Show the bell icon in red color;  -> Right
    
  • Show counter in red color;  -> Right 
    
  • The counter will be unseen or unread count? -> Unseen
    

Also, for the Completed Challenges we would like to limit their number that are displayed in the popup to 5. We can show the rest of them in the Notifications Details page.

@Oanh-and-only-Oanh Oanh-and-only-Oanh removed the QA Fail QA verification on Dev has failed. Assignee to redo the fix. label Feb 27, 2020
@SathyaJayabal
Copy link
Collaborator

SathyaJayabal commented Mar 3, 2020

@luizrrodrigues , all the issues above have been addressed.

New Requirements that need to be implemented:
Remove the completed challenges section from the popup.

Clarification:
@luizrrodrigues , the challenge notification should take the user to the. challenge details page on clicking it. Will this be implemented after api integration ?

@SathyaJayabal SathyaJayabal added QA Fail QA verification on Dev has failed. Assignee to redo the fix. Test Env Environment and removed Ready for QA QA Fail QA verification on Dev has failed. Assignee to redo the fix. labels Mar 3, 2020
@luizrrodrigues
Copy link
Collaborator

Clarification: the challenge notification should take the user to the. challenge details page on clicking it. Will this be implemented after API integration?

@SathyaJayabal Yes, we need to check as the backend will pass the challenge link.

@macs054
Copy link
Collaborator

macs054 commented Mar 10, 2020

Verified in Test Env

@SathyaJayabal
Copy link
Collaborator

Bell icon is in active state but the notifications popup and the notifications page do not display any notifications
Screenshot 2020-03-26 at 5 21 41 PM

@luizrrodrigues , this could be because of the older submission uploaded notifications returned by the backend, but not shown by the front end.

@SathyaJayabal SathyaJayabal added Beta Env Environment QA Fail QA verification on Dev has failed. Assignee to redo the fix. and removed QA Pass labels Mar 26, 2020
@SathyaJayabal
Copy link
Collaborator

cc @sachin-maheshwari

@luizrrodrigues
Copy link
Collaborator

luizrrodrigues commented Mar 26, 2020

@SathyaJayabal Yep, we need to remove this from backend return.

I think we can close this one and follow up here: #4108

@SathyaJayabal
Copy link
Collaborator

Sure @luizrrodrigues

@SathyaJayabal SathyaJayabal added QA Pass and removed QA Fail QA verification on Dev has failed. Assignee to redo the fix. labels Mar 27, 2020
@SathyaJayabal SathyaJayabal removed the Need clarification Need clarification to proceed fixing the issue further label Mar 27, 2020
@SathyaJayabal SathyaJayabal added this to the v0.27.0 milestone Mar 30, 2020
@SathyaJayabal SathyaJayabal removed the Test Env Environment label Mar 30, 2020
@SathyaJayabal
Copy link
Collaborator

verified on beta(develop)
Screenshot 2020-03-30 at 1 55 57 PM

@SathyaJayabal SathyaJayabal added Prod Env Environment and removed Beta Env Environment labels Mar 30, 2020
@SathyaJayabal
Copy link
Collaborator

verified on prod
Screenshot 2020-03-30 at 5 53 01 PM

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

No branches or pull requests

7 participants