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

[C-4453] Fix notification email image and layout #8647

Merged
merged 3 commits into from
May 30, 2024

Conversation

isaacsolo
Copy link
Contributor

@isaacsolo isaacsolo commented May 29, 2024

Description

Notification emails look wonky because:

  • Storage V2 stores images in a different URL. We can let the selected content node handle rendezvous.
  • Responsive mobile Safari / Apple Mail changes the layout. Removing this text-align fixes.

IMG_3770

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

Ran with stage DN1. Confirmed layout looks good on web and mobile Chrome / Safari.

IMG_3769

Copy link

changeset-bot bot commented May 29, 2024

⚠️ No Changeset found

Latest commit: 9bb7b91

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@isaacsolo isaacsolo force-pushed the is-fix-notif-email-layout branch from 8447c49 to 5643993 Compare May 29, 2024 22:35
@isaacsolo isaacsolo changed the title Fix notification email image and layout [C-4453] Fix notification email image and layout May 29, 2024
Copy link
Contributor

@dylanjeffers dylanjeffers left a comment

Choose a reason for hiding this comment

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

nice job, mind running a quick lint/lint:fix?

if (!user.creator_node_endpoint) {
return null
}
const contentNodes = user.creator_node_endpoint.split(',')
Copy link
Contributor

Choose a reason for hiding this comment

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

big yikes. nice. can you check if user has .cover_photo and .profile_picture ? if they are coming from our api they should just full content-node urls

Copy link
Contributor Author

@isaacsolo isaacsolo May 29, 2024

Choose a reason for hiding this comment

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

don't think we have those, it's coming straight from the discovery DB

const userRows: UserResource[] = await dnDb
.select(
'users.user_id',
'users.handle',
'users.name',
'users.profile_picture_sizes',
'users.profile_picture',
'users.creator_node_endpoint'
)
.from('users')
.whereIn('user_id', Array.from(ids.users))
.andWhere('is_current', true)

Copy link
Contributor

Choose a reason for hiding this comment

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

ah word, yup this good then :)

@pull-request-size pull-request-size bot added size/L and removed size/M labels May 30, 2024
@isaacsolo isaacsolo merged commit a495f42 into main May 30, 2024
11 of 12 checks passed
@isaacsolo isaacsolo deleted the is-fix-notif-email-layout branch May 30, 2024 22:02
audius-infra pushed a commit that referenced this pull request May 31, 2024
[7343561] [PAY-2842][PAY-2780][PAY-3069] Fix authorized endpoints not supporting manager mode (#8646) Randy Schott
[ddaa1b3] ⚠️ Third Party Wallet Support [PAY-2949][PAY-2948][PAY-2950] (#8611) Marcus Pasell
[c15f20d] Fix prod mediorum dockerfile to have keyfinder dependency (#8666) Michelle Brier
[7fe9451] Fix solana-relay logging, fix eslint config (#8663) Marcus Pasell
[a495f42] [C-4453] Fix notification email image and layout (#8647) Isaac Solo
[2ceae28] Audio analysis: mediorum changes (#8536) Michelle Brier
[15a34b3] [C-4353] Add recent searches to search v2 (#8615) Sebastian Klingler
[0050b29] Fix sdk typecheck (#8656) Sebastian Klingler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants