Skip to content
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

Add notifications drawer #4210

Merged
merged 32 commits into from
Jul 19, 2024
Merged

Add notifications drawer #4210

merged 32 commits into from
Jul 19, 2024

Conversation

cstns
Copy link
Contributor

@cstns cstns commented Jul 18, 2024

Description

Adds a notifications drawer boilerplate. Currently only supports team invitations but is easily extended to future notifications

Related Issue(s)

closes #4184

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

@cstns cstns requested a review from joepavitt July 18, 2024 16:28
@cstns cstns self-assigned this Jul 18, 2024
Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.64%. Comparing base (168eb19) to head (a7f8528).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4210   +/-   ##
=======================================
  Coverage   78.64%   78.64%           
=======================================
  Files         286      286           
  Lines       13119    13119           
  Branches     2934     2934           
=======================================
  Hits        10317    10317           
  Misses       2802     2802           
Flag Coverage Δ
backend 78.64% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cstns cstns changed the title 4184 Add notifications drawer Add notifications drawer Jul 18, 2024
Copy link
Contributor

@joepavitt joepavitt left a comment

Choose a reason for hiding this comment

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

  • Odd glitch on close where it seems to span the full width for a brief millisecond or two, then pull back?
Screen.Recording.2024-07-18.at.18.16.07.mov
  • I'd also expect to be able to click the notifications icon again to close too, not just have to click into empty space.
  • We should also change the red of the ff-notification-pill to #d12c2b as it's too dark at the moment
  • We also need to improve the padding and vertical alignment of the notifications themselves:
Screenshot 2024-07-18 at 18 20 49

I'm happy to do that myself though if you don't mind handling the other recommendations

frontend/src/components/NotificationsButton.vue Outdated Show resolved Hide resolved
frontend/src/components/NotificationsButton.vue Outdated Show resolved Hide resolved
Co-authored-by: Joe Pavitt <99246719+joepavitt@users.noreply.github.com>
Co-authored-by: Joe Pavitt <99246719+joepavitt@users.noreply.github.com>
@joepavitt
Copy link
Contributor

joepavitt commented Jul 18, 2024

As an FYI too, we have ff-icon, ff-icon-lg and ff-icon-sm as general utility classes which you can use here too. The icon in the notification itself for example can just use ff-icon ff-icon-lg and it'll then be 24px squared.

@cstns
Copy link
Contributor Author

cstns commented Jul 18, 2024

I'm looking into the collapse bug! Feel free to make any adjustments you feel necessary!

…xes the right drawer opening and closing when clicking the notification button with the drawer open
@joepavitt
Copy link
Contributor

joepavitt commented Jul 19, 2024

Made some structural improvmenets by having all of the structure live inside a Notification.vue component with relevant slots that the bespoke notifications can extend.

Also modified the click behaviour to make the whole notification clickable (may need to revert with the option to "check" notifications later)

I've also then modified the CSS slightly to implement the following changes:

Before

Screenshot 2024-07-19 at 12 36 54

After

Screenshot 2024-07-19 at 12 34 07

@joepavitt
Copy link
Contributor

Waiting for tests to run - but will approve once they're successful

@joepavitt joepavitt merged commit 940a813 into main Jul 19, 2024
14 checks passed
@joepavitt joepavitt deleted the 4184_notifications-inbox-view branch July 19, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Front-End: Add Notifications Inbox view
2 participants