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

notifications for billing #1656

Merged
merged 9 commits into from
Nov 3, 2021
Merged

Conversation

tanmoyAtb
Copy link
Contributor

@tanmoyAtb tanmoyAtb commented Oct 25, 2021

closes #1646

Edit this in AppHeader.tsx to see some notifications.

<NotificationsDropdown
  notifications={[
     {
         title: "This is a title. Add as many lines as you can",
         createdAt: new Date()
     }
   ]}
/>

Note: Notifications button don't actually resemble the designs, the designs have the new design of buttons and UI elements which we have to work on.

@render
Copy link

render bot commented Oct 25, 2021

@github-actions github-actions bot added the Type: Feature Added to PRs to identify that the change is a new feature. label Oct 25, 2021
@lgtm-com
Copy link

lgtm-com bot commented Oct 25, 2021

This pull request introduces 1 alert when merging 58e486d into c6e935b - view on LGTM.com

new alerts:

  • 1 for Identical operands

@tanmoyAtb tanmoyAtb marked this pull request as ready for review October 29, 2021 16:54
@tanmoyAtb tanmoyAtb requested review from RyRy79261 and Tbaut October 29, 2021 16:54
@tanmoyAtb tanmoyAtb assigned FSM1 and unassigned FSM1 Oct 29, 2021
@tanmoyAtb tanmoyAtb requested a review from FSM1 October 29, 2021 16:55
@lgtm-com
Copy link

lgtm-com bot commented Oct 29, 2021

This pull request introduces 1 alert when merging 57e5ae9 into c6e935b - view on LGTM.com

new alerts:

  • 1 for Identical operands

@tanmoyAtb tanmoyAtb added Status: Review Needed 👀 Added to PRs when they need more review and removed Type: Feature Added to PRs to identify that the change is a new feature. labels Oct 29, 2021
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Look pretty good. Just a couple nits. Also we should have some max height and scroll bar

image

Also how should this look like on Mobile @serenaho and @sweetpea22

tanmoyAtb and others added 3 commits November 1, 2021 18:16
…n.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 1, 2021

This pull request introduces 1 alert when merging 2133e1b into c6e935b - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@sweetpea22
Copy link
Contributor

The notification will replace the search icon let me pull up the design brb!

@tanmoyAtb
Copy link
Contributor Author

Look pretty good. Just a couple nits. Also we should have some max height and scroll bar

image

Also how should this look like on Mobile @serenaho and @sweetpea22

Thanks for the feedbacks,
I made the updates and added a scrollbar, thats was a good point.

@tanmoyAtb tanmoyAtb requested a review from Tbaut November 1, 2021 18:28
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Looks good, let's just not push with those fixture notifications :)

packages/files-ui/src/Components/Layouts/AppHeader.tsx Outdated Show resolved Hide resolved
Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@Tbaut Tbaut merged commit 766facb into epic/files-billing Nov 3, 2021
@Tbaut Tbaut deleted the feat/notifications-1646 branch November 3, 2021 13:21
@FSM1 FSM1 removed the Status: Review Needed 👀 Added to PRs when they need more review label Nov 25, 2021
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.

5 participants