#663 notifications - layout only #934

sandrina-p · 2020-05-30T13:35:12Z

Starting #663

What's done:

All the layout with dummy data. Focus only on its visual aspect. Don't expect any info to be correct (e.g. names, avatars, dates, etc...) That's for another PR.

Notes:

For now, the dummy data is based on the number of members in a group:
- 1 member: No notifications.
- 2 members: 1 notification.
- +2 members: 7 notifications.
@mmbotelho I left some questions in Figma about the layout. Once answered, I'll update this PR.
@mmbotelho, if you use the keyboard to open/navigate through the notification card, eventually the close button will show up (similar to the profile card). But its placement is not the best one. What do you suggest?
Ignore all the code related to logic. That's for another PR.
I think we should do two video calls about this topic:
- Review each notification message in detail (copy, type, icons, etc...) and analyze possible technical restrictions for the prototype.
- Discuss the notification system architecture (logic, backend, etc...)

Bugs found (to be fixed in other PR):

Open the notifications card/modal, click "Settings". The User Settings modal won't open at the "Notifications" section.

mmbotelho

Here's what I found @sandrina-p :

mmbotelho · 2020-06-10T17:28:15Z

@mmbotelho I left some questions in Figma about the layout. Once answered, I'll update this PR.

I looked at Figma and I believe I answered everything. Let me know if I missed something!

@mmbotelho, if you use the keyboard to open/navigate through the notification card, eventually the close button will show up (similar to the profile card). But its placement is not the best one. What do you suggest?

I suggest leaving it as it is (as a fallback) but having the ESC key closing the popup as well.

Review each notification message in detail (copy, type, icons, etc...) and analyze possible technical restrictions for the prototype.
I'm not sure if you saw this, but I made a table with all the messages I could think of. Let me know what's missing! We can also schedule a call, whatever works best for you!

sandrina-p · 2020-06-13T09:56:37Z

@mmbotelho

Extra gray spaces fixed. This was a tricky one to understand. It's the scrollbar, visible by default (I forget that this is the default setting on windows). Now, all scrollbars should be hidden when not needed.
Alignments fixed.
Yes, I saw that list and I still think it's incomplete or it should have a new review. For example "Pending payment confirmations" does not make sense anymore. And maybe some of these notifications should be similar to the emails list... Anyway, that's not for this PR. This is just dummy layout for now. Later, when @taoeffect has more time, we can talk about this.

pieer · 2020-06-15T04:25:53Z

frontend/views/components/modal/Modal.vue

@@ -110,7 +110,8 @@ export default {
        this.content = null
        this.contentProps = {}
        // Refocus on button that open the modal
-        this.lastFocus.focus()
+        // TODO/BUG It might not exist... (e.g. profile card was closed). What's the best approach then?


The focus should be on the button that open the modal. In case of modal opening a modal, meaning that the last focus element is not present, the best approach I guess is to focus on the button that open the first modal. We can do that by recording the previous focused element if the last one doesn't exist. Or a simpler solution is to go back to the first focusable element in the page

Here the trick is the profile|notifications cards. For example, clicking "Remove Member" in the profile card, it opens the modal and closes the card. This means that this.lastFocus does not exist anymore. When we close the modal, it will go back to the top of the page because of that. But it should go back to the respective user in the members' list.
I didn't find any clean way to fix this... but I'll keep in the back of my head

frontend/views/containers/notifications/NotificationsBell.vue

frontend/views/containers/notifications/NotificationsCard.vue

frontend/views/containers/notifications/NotificationsModal.vue

mmbotelho · 2020-06-15T09:21:20Z

Yes, I saw that list and I still think it's incomplete or it should have a new review. For example "Pending payment confirmations" does not make sense anymore.

@sandrina-p you're right! My bad, I forgot a lot of things changed since I wrote that list! Definitely needs to be reviewed

sandrina-p · 2020-06-15T14:52:53Z

Okay, @mmbotelho. Later in the road, maybe after #916 and #918 are merged, we can discuss this again. ATM we are already stretching our "multi-tasking work" with too many tasks at the same time.

mmbotelho · 2020-06-19T16:22:59Z

I agree @sandrina-p !

mmbotelho

Only 2 things missing:

thesoftwarephilosopher

Found the fix to the alignment issue above. Otherwise, looks good!

frontend/views/containers/notifications/NotificationsList.vue

frontend/model/notifications/templates.js

taoeffect · 2020-09-22T19:16:30Z

Closing in favor of squashed version #995. Review continues there and the PR will be picked up by @F1LT3R.

sandrina-p requested review from taoeffect, pieer and mmbotelho May 30, 2020 13:35

sandrina-p force-pushed the task/663-notifications_layout branch 2 times, most recently from d517868 to 3d8db02 Compare May 31, 2020 09:48

mmbotelho requested changes Jun 10, 2020

View reviewed changes

pieer requested changes Jun 15, 2020

View reviewed changes

sandrina-p force-pushed the task/663-notifications_layout branch 2 times, most recently from 01b3318 to 10bd9ee Compare June 16, 2020 08:11

mmbotelho self-requested a review July 20, 2020 14:44

mmbotelho requested changes Jul 20, 2020

View reviewed changes

taoeffect added the Priority:High label Sep 8, 2020

thesoftwarephilosopher suggested changes Sep 8, 2020

View reviewed changes

frontend/views/containers/notifications/NotificationsList.vue Outdated Show resolved Hide resolved

taoeffect mentioned this pull request Sep 9, 2020

Check the project for XSS [$250 bounty] #975

Closed

taoeffect reviewed Sep 9, 2020

View reviewed changes

frontend/model/notifications/templates.js Show resolved Hide resolved

sandrina-p force-pushed the task/663-notifications_layout branch from 10bd9ee to a2f23b5 Compare September 12, 2020 07:21

sandrina-p added 9 commits September 15, 2020 17:26

Implement Notifications Trigger, Card and Modal - desktop

9e458d8

NotificationsList - layout

7829c6f

Create fakeDB with templates

9e13fd2

Add link "See All"

7f69f55

Continue Notifications layout

cb65aaf

Continue Notifications

3016228

add badge on mobile

c08b14a

Implement timeSince() with tests

2940462

Hide scrollbars when non-existent.

97525c4

sandrina-p and others added 3 commits September 15, 2020 17:26

Notification title "new" spacing tweak

ffc8d8b

Changes based on @mmbotelho review

e02a58c

Changes based on @mmbotelho review

1f7a6fd

sandrina-p force-pushed the task/663-notifications_layout branch from a2f23b5 to 1f7a6fd Compare September 15, 2020 16:28

taoeffect mentioned this pull request Sep 22, 2020

Implement Notifications Trigger, Card and Modal - desktop #994

Closed

taoeffect closed this Sep 22, 2020

taoeffect mentioned this pull request Sep 22, 2020

Task/663 notifications layout 3 #995

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#663 notifications - layout only #934

#663 notifications - layout only #934

sandrina-p commented May 30, 2020 •

edited

Loading

mmbotelho left a comment

mmbotelho commented Jun 10, 2020

sandrina-p commented Jun 13, 2020

pieer Jun 15, 2020

sandrina-p Jun 16, 2020

mmbotelho commented Jun 15, 2020

sandrina-p commented Jun 15, 2020

mmbotelho commented Jun 19, 2020

mmbotelho left a comment

thesoftwarephilosopher left a comment

taoeffect commented Sep 22, 2020 •

edited

Loading

#663 notifications - layout only #934

#663 notifications - layout only #934

Conversation

sandrina-p commented May 30, 2020 • edited Loading

What's done:

Notes:

Bugs found (to be fixed in other PR):

mmbotelho left a comment

Choose a reason for hiding this comment

mmbotelho commented Jun 10, 2020

sandrina-p commented Jun 13, 2020

pieer Jun 15, 2020

Choose a reason for hiding this comment

sandrina-p Jun 16, 2020

Choose a reason for hiding this comment

mmbotelho commented Jun 15, 2020

sandrina-p commented Jun 15, 2020

mmbotelho commented Jun 19, 2020

mmbotelho left a comment

Choose a reason for hiding this comment

thesoftwarephilosopher left a comment

Choose a reason for hiding this comment

taoeffect commented Sep 22, 2020 • edited Loading

sandrina-p commented May 30, 2020 •

edited

Loading

taoeffect commented Sep 22, 2020 •

edited

Loading