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

[PAY-1687] Web: Share tracks, playlists, and albums via Direct Message - #3828

Merged
rickyrombo merged 9 commits into
mainfrom
mjp-dms-share-track
Aug 2, 2023
Merged

[PAY-1687] Web: Share tracks, playlists, and albums via Direct Message#3828
rickyrombo merged 9 commits into
mainfrom
mjp-dms-share-track

Conversation

@rickyrombo

@rickyrombo rickyrombo commented Aug 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Adds support for sharing track tiles, playlist tiles, and album tiles via direct messages. Also adds embed to share modal.

Screen.Recording.2023-08-01.at.1.16.11.PM.mov

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

How Has This Been Tested?

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

Tested locally against stage

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

@audius-infra

audius-infra commented Aug 1, 2023

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/mjp-dms-share-track

dharit-tan
dharit-tan approved these changes Aug 1, 2023
View reviewed changes

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

would be nice to get a screencap of this in action!

Comment thread packages/web/src/components/share-modal/ShareModal.tsx
Comment thread packages/web/src/pages/chat-page/components/ChatComposer.tsx Outdated
@rickyrombo

rickyrombo commented Aug 1, 2023

Copy link
Copy Markdown
Contributor Author

@dharit-tan added screencap!

piazzatron
piazzatron approved these changes Aug 1, 2023
View reviewed changes

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

Code looks great!! A few visual things I noticed:

  • Header doesn't appear centered, icon spacing doesn't match figma (nor does color/capitalization, but not sure what 'version' of modal titles we're on here)
Screen Shot 2023-08-01 at 5 48 07 PM
  • Hover state for these buttons is very large (maybe this is a harmony thing, and we punt for now):
Screen Shot 2023-08-01 at 5 48 43 PM

  • Spacing above and below "spread the word" CTA doesn't match figma (less space in figma)

  • Modal transitions: the compose modal pops up below the share modal, before the share model closes, which feels pretty jarring to me. Any we to ensure the share model closes before the compose modal appears?

Then one last UX nit, which we shouldn't do if is tricky – when we drop into the chats page from this share flow, would be nice if the text input automatically had focus so you can just press enter to send your track

@rickyrombo

rickyrombo commented Aug 2, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@piazzatron :

  1. Modal Header: Gonna punt as I think we'll want to do a full modal pass to get all modals to match that style (this has been a mismatch between figma and production for a while, so I assume safe to ignore)
  2. Button hover: will leave as-is and wait for design feedback
  3. Spacing above and below: Good catch, fixed
  4. Modal transitions: There's no easy clean way to do this afaik. Simplest thing would be to put the Share modal under the chat create modal? I can play with it in a follow up but I don't think we should block on this. I think modals generally need a bit of an overhaul to support a modal stack, I'm not in love with my "cancel" hack
  5. Autofocus: Will see if I can address separately fixed

@rickyrombo rickyrombo changed the title [PAY-1665] Web: Share tracks, playlists, and albums via Direct Message [PAY-1687] Web: Share tracks, playlists, and albums via Direct Message Aug 2, 2023
@audius-infra

audius-infra commented Aug 2, 2023

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/mjp-dms-share-track

@rickyrombo
rickyrombo merged commit df90566 into main Aug 2, 2023
@rickyrombo
rickyrombo deleted the mjp-dms-share-track branch August 2, 2023 00:59
@rickyrombo rickyrombo mentioned this pull request Aug 2, 2023
Merged
schottra added a commit that referenced this pull request Aug 3, 2023
@schottra
Merge remote-tracking branch 'origin/main' into pay-1630-purchase-sagas
101e87a 
* origin/main:
  Add npm run clean script (#3846)
  DMs: Web: Don't nav back when clicking outside the modal (#3844)
  Add prepare step to dapp-store ci flow (#3841)
  [Harmony] Add SelectablePill to Harmony PAY-1654 (#3803)
  [PAY-1688] Mobile: Share track, collection to DMs (#3840)
  Disable upload redesign (#3842)
  Update dapp-store build artifacts
  Update dapp-store build artifacts
  Fix dapp store deployment (#3829)
  [C-2907] Add contextual-menu, refactor release-date-field (#3836)
  [PAY-1687] Web: Share tracks, playlists, and albums via Direct Message (#3828)
  Upgrade sdk to beta.105 to fix rewards claiming (#3839)
  [C-2923] Fix toasts in modal screens (#3838)
  Add cypress upload test for subgenre (#3833)
  [PAY-1685] Wire up stripe UI for USDC purchase in mobile (#3837)
  Fix broken track upload for electronic subgenres on mobile (#3835)
  Fix tag input (#3832)
audius-infra pushed a commit that referenced this pull request Aug 5, 2023
audius-infra
v1.5.36
bb99113 
[f9e1380] Add DirectMessages Banner and Update All Banners (#3851) Marcus Pasell
[e523d39] [PAY-1692] Rewrite 'Share to DMs' using less stateful logic (#3852) Marcus Pasell
[af62892] [C-2675][C-2692] Add multi track navigation sidebar and form controls (#3847) Andrew Mendelsohn
[ee38400] Fix send audio flow (#3850) Reed
[bd1752b] Update SDK to latest 3.0.3-beta.109 (#3849) nicoback2
[6988a03] Add npm run clean script (#3846) Reed
[68bd638] DMs: Web: Don't nav back when clicking outside the modal (#3844) Marcus Pasell
[1f7d7d0] Add prepare step to dapp-store ci flow (#3841) Raymond Jacobson
[6c87efd] [Harmony] Add SelectablePill to Harmony PAY-1654 (#3803) nicoback2
[69fbfb3] [PAY-1688] Mobile: Share track, collection to DMs (#3840) Marcus Pasell
[f2f9717] Disable upload redesign (#3842) Andrew Mendelsohn
[9671c06] Update dapp-store build artifacts audius-infra
[77b4030] Update dapp-store build artifacts audius-infra
[baedc13] Fix dapp store deployment (#3829) Raymond Jacobson
[bcb5137] [C-2907] Add contextual-menu, refactor release-date-field (#3836) Dylan Jeffers
[df90566] [PAY-1687] Web: Share tracks, playlists, and albums via Direct Message (#3828) Marcus Pasell
[ec16cc3] Upgrade sdk to beta.105 to fix rewards claiming (#3839) Dylan Jeffers
[270cc2f] [C-2923] Fix toasts in modal screens (#3838) Dylan Jeffers
[18616df] Add cypress upload test for subgenre (#3833) Raymond Jacobson
[8a76f7c] [PAY-1685] Wire up stripe UI for USDC purchase in mobile (#3837) Reed
[6337949] Fix broken track upload for electronic subgenres on mobile (#3835) nicoback2
[f643695] Fix tag input (#3832) Andrew Mendelsohn
[5da8e8f] Update userbank function usage to pass config object (#3823) Randy Schott
[8a5419d] Update dapp-store build artifacts audius-infra
[6284c14] [C-2857] Revert remove get blocknumber (#3802)" (#3826) Dylan Jeffers
[6a54b3a] [C-2742] Multi-track form pagination (#3818) Andrew Mendelsohn
[65471a1] Bump mobile versions for client v1.5.35 full app release (#3827) nicoback2
[6c88515] Revert "Add purchased + reposted tracks to library PAY-1633  (#3820)" (#3825) nicoback2
[8dc4c8a] Add purchased + reposted tracks to library PAY-1633  (#3820) nicoback2
[5ddf078] Update SDK version + ActivityFull type (#3819) nicoback2
[c643b8a] Use audius-query in USDC Purchase Drawer (#3822) Reed
[a119db1] Update bootstrap nodes (#3821) Theo Ilie
[d2630a8] [PAY-1589] Wire up Stripe Onramp in mobile (#3814) Reed
@AudiusProject AudiusProject deleted a comment from linear Bot Sep 11, 2023
@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.

Reviewers

2 more reviewers
@dharit-tan dharit-tan dharit-tan approved these changes
@piazzatron piazzatron piazzatron approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@schottra schottra

@dylanjeffers dylanjeffers

@sddioulde sddioulde

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@rickyrombo @audius-infra @dharit-tan @piazzatron @schottra @dylanjeffers @sddioulde