Skip to content
This repository was archived by the owner on Oct 4, 2023. It is now read-only.

[PAY-166] Redesign Repost Notification#1276

Merged
dylanjeffers merged 4 commits into
mainfrom
dj-pay-166-redesign-repost-notification
May 3, 2022
Merged

[PAY-166] Redesign Repost Notification#1276
dylanjeffers merged 4 commits into
mainfrom
dj-pay-166-redesign-repost-notification

Conversation

@dylanjeffers

Copy link
Copy Markdown
Contributor

Description

Redesigns Repost Notification

  • Adds new RepostNotification component
  • Implements reuseable EntityLink component
  • Implements unified icon styles
  • Adds reuseable TwitterShareButton

@audius-infra

Copy link
Copy Markdown
Collaborator

@rickyrombo rickyrombo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might make things easier to review if you make the base of this PR the branch for the other PR - then if the other one goes first you can rebase on main and set this PR back to main

EDIT: JK I see there's only two commits, looking at the second commit now 😄

@dylanjeffers

Copy link
Copy Markdown
Contributor Author

Thank you for solving one of my problems for a while now, will do

@rickyrombo rickyrombo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Image preview of a repost notification 👀?


import styles from './EntityLink.module.css'

type EntityType = (Collection | Track) & { user: User }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a feeling this may need to support Tips and Users in the future 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah that's the hope :D

}

export const getEntityLink = (
entity: (Track & { user: User }) | (Collection & { user: User }),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Feels like we should break NotificationEntity into its own type

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah should be reusing EntityType, good callout

@dylanjeffers dylanjeffers changed the base branch from main to dj-pay-161-desktop-notification-redesign May 3, 2022 17:28
@pull-request-size pull-request-size Bot added size/L and removed size/XL labels May 3, 2022
Base automatically changed from dj-pay-161-desktop-notification-redesign to main May 3, 2022 18:16
@pull-request-size pull-request-size Bot added size/XL and removed size/L labels May 3, 2022
@pull-request-size pull-request-size Bot added size/L and removed size/XL labels May 3, 2022
@audius-infra

Copy link
Copy Markdown
Collaborator

@dylanjeffers dylanjeffers merged commit ec671b2 into main May 3, 2022
@dylanjeffers dylanjeffers deleted the dj-pay-166-redesign-repost-notification branch May 3, 2022 19:22
@sliptype sliptype mentioned this pull request May 13, 2022
@AudiusProject AudiusProject deleted a comment from linear Bot Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants