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

[PAY-161] Redesign Follow Notification#1273

Merged
dylanjeffers merged 2 commits into
mainfrom
dj-pay-161-desktop-notification-redesign
May 3, 2022
Merged

[PAY-161] Redesign Follow Notification#1273
dylanjeffers merged 2 commits into
mainfrom
dj-pay-161-desktop-notification-redesign

Conversation

@dylanjeffers

@dylanjeffers dylanjeffers commented Apr 30, 2022

Copy link
Copy Markdown
Contributor

Description

Redesigns Desktop follow notifications using reusable Notification components.

  • Adds FollowNotification
  • Adds NotificationTile, NotificationHeader, NotificationBody, and NotificationFooter components

image

@dylanjeffers dylanjeffers force-pushed the dj-pay-161-desktop-notification-redesign branch from 8f26c79 to 20b71bd Compare April 30, 2022 01:20
@audius-infra

Copy link
Copy Markdown
Collaborator

@dylanjeffers dylanjeffers requested a review from rickyrombo May 2, 2022 20:48
@dylanjeffers dylanjeffers changed the title [PAY-161] Redesign desktop follow notification [PAY-161] Redesign Follow Notification May 3, 2022

@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.

Love the new NotificationTile/NotificationHeader/NotificationBody/NotificationFooter approach it's beautiful

Can I play with this somewhere 👀

if (!loadImage) {
const t = setTimeout(() => {
setLoadImage(true)
}, 500)

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.

Love this optimizing! Although half a second is pretty long, can it be shorter? also should this be a constant at the top?

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.

pretty much pulled from the existing notifs image, happy to do that

import React, { MouseEventHandler, useCallback } from 'react'

import cn from 'classnames'
import { push } from 'connected-react-router'

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.

super-duper nit but I think we typically import this as pushRoute for clarity?

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 sure thing

if (is_deactivated) {
return (
<span className={cn(styles.root, styles.deactivated)}>
{name} [{messages.deactivated}]

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.

<3 the joy of deactivated accounts


let userNameElement = (
<span className={styles.root}>
<a onClick={handleClick} href={`/${handle}`} className={styles.link}>

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.

Maybe reuse profilePage() here for the href

users: Array<User>
}

export const UserProfileList = ({ users }: UserProfileListProps) => {

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.

Nit: Maybe UserProfilePictureList?

<div className={styles.root}>
{users
.filter(u => !u.is_deactivated)
.slice(0, showUserListModal ? USER_LENGTH_LIMIT : USER_LENGTH_LIMIT)

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.

What is this ternary doing?

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.

good catch, nothing now!

@dylanjeffers dylanjeffers requested a review from sddioulde May 3, 2022 16:50
@audius-infra

Copy link
Copy Markdown
Collaborator

@dylanjeffers dylanjeffers merged commit 8e9c5e4 into main May 3, 2022
@dylanjeffers dylanjeffers deleted the dj-pay-161-desktop-notification-redesign branch May 3, 2022 18:16
}

export const UserProfilePictureList = ({ users }: UserProfileListProps) => {
const showUserListModal = users.length > USER_LENGTH_LIMIT

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.

we could make USER_LENGTH_LIMIT a prop so that diff places (e.g. top followers in tipping) using this list can control what the cutoff is

disablePopover?: boolean
}

export const ProfilePicture = (props: ProfilePictureProps) => {

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.

hmm looks like i should be able to use this component in many of the tipping components im making. nice

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.

Sweet, I'll move both of these to components directory in the next few prs

@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.

4 participants