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

[PAY-507] Fix feed tip tile tip button resizing #1837

Merged
merged 3 commits into from
Sep 1, 2022
Merged

Conversation

dharit-tan
Copy link
Contributor

@dharit-tan dharit-tan commented Aug 31, 2022

Description

Fixes feed tip tile "send tip" button resizing issue. Now also wraps the recipient info and "was tipped by" section.

Dragons

Some jumpiness if the user resizes the window quickly, see the end of the video.

Now

How Has This Been Tested?

Tested on local web

Screen.Recording.2022-09-01.at.12.05.26.AM.mov

How will this change be monitored?

Feature Flags

<SendTipButton
user={usersMap[tipToDisplay.receiver_id]}
hideName={useShortButtonFormat}
hideName={useShortButtonFormat.current}
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of all the JS logic, can we add CSS media queries for various screen size breakpoints? That's typically how responsive ui is done afaik

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that was how we were doing it before but that's where the problem came from. The resize really depends on the width of the other contents of the tile, not the screen size. There's probably a way we can determine whether to resize by incorporating the screen size, but I don't think that'll save us from having to do these calculations?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I see, we're not resizing based on window width alone, but based on widths of the button + the other content being >= the container?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. Tho I could use those numbers to calculate the window size at which we'd need to resize if that's preferred.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I get it and what you have seems right...

Sorry for my confusion, I'm ramping up to speed here - there's two things happening from what I see:

  1. The button is changing at the component level based on how much space it has - this is what you've solved here with measuring the size of the two elements and the container and passing a prop to the button. This makes sense because the goal is to change the button based on how long the names are, not how big the screen is - otherwise we could use CSS media queries here to hide the "to (username)" elements.
  2. The content is overflowing when it shouldn't on small screens even after the smaller button - this is a "responsive" bug and is going to need some flex-shrink, flex-wrap, or CSS media queries. This is out of scope for the PR

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/rt-PAY-507

<SendTipButton
user={usersMap[tipToDisplay.receiver_id]}
hideName={useShortButtonFormat}
hideName={useShortButtonFormat.current}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I get it and what you have seems right...

Sorry for my confusion, I'm ramping up to speed here - there's two things happening from what I see:

  1. The button is changing at the component level based on how much space it has - this is what you've solved here with measuring the size of the two elements and the container and passing a prop to the button. This makes sense because the goal is to change the button based on how long the names are, not how big the screen is - otherwise we could use CSS media queries here to hide the "to (username)" elements.
  2. The content is overflowing when it shouldn't on small screens even after the smaller button - this is a "responsive" bug and is going to need some flex-shrink, flex-wrap, or CSS media queries. This is out of scope for the PR

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/rt-PAY-507

@dharit-tan dharit-tan merged commit 779d9e9 into main Sep 1, 2022
@dharit-tan dharit-tan deleted the rt-PAY-507 branch September 1, 2022 18:08
audius-infra pushed a commit that referenced this pull request Sep 3, 2022
[6eeb9d4] [PAY-605][PAY-601][PAY-548] Buy Audio modal design changes (#1846) Marcus Pasell
[436cd27] [C-963] Migrate wallet sagas (#1857) Sebastian Klingler
[238d729] [C-959] Migrate tipping sagas (#1852) Sebastian Klingler
[1561728] Bump SDK to v1.0.1 (#1842) Dheeraj Manjunath
[a331f3d] Add reordering to new tables for playlists (#1845) Kyle Shanks
[3c90d3f] [PAY-598] Prepopulate TransactionDetails after purchasing $AUDIO (#1832) Marcus Pasell
[b11a686] [C-957] Fix autocomplete when signed out (#1844) Sebastian Klingler
[3315c27] Make tooltips use updated colors when theme is applied (#1843) Sebastian Klingler
[76395d5] [PAY-591] Improve supporter count visibility (#1841) Reed
[779d9e9] [PAY-507] Fix feed tip tile tip button resizing (#1837) Reed
[f4160e9] [C-939] Refactor discord modal (#1838) Dylan Jeffers
[b76854f] Update deactivate copy (#1840) Raymond Jacobson
[2085ffc] [Stems] Don't rewrite to /index.html on stems.audius.co (#1836) Marcus Pasell
[14aa5ac] [C-836] Fix enter on search (#1835) Dylan Jeffers
[d644cc1] [C-703] Fix segmented control font color (#1833) Dylan Jeffers
[d2c0dca] [C-934] Migrate user-list sagas to common (#1829) Dylan Jeffers
[1321a57] [C-919] Migrate profile sagas to common (#1826) Sebastian Klingler
[aad5c6a] [PAY-594] Edit 'not enough SOL' error message (#1827) Reed
[af61aab] [C-930] Move social sagas to common (#1822) Raymond Jacobson
[33a1403] [C-928] Migrate saved sagas to common (#1821) Dylan Jeffers
[64d45f3] [PAY-544] Add Transaction Details Modal (#1806) Marcus Pasell
[20cfc9f] Add Transaction Details to Store (#1804) Marcus Pasell
[5b8a4f0] [C-926] Migrate explore sagas to common (#1818) Dylan Jeffers
[6047185] Follow up fix for track removal from playlist with entity manager (#1817) Isaac Solo
[1b2638c] Fix track removal from playlist with entity manager (#1815) Isaac Solo
[c854f66] [C-918] Move trending saga to common (#1814) Dylan Jeffers
[6f24724] Fix lineup types in mobile build (#1812) Dylan Jeffers
[27cf13a] Update @audius/sdk to 1.0.0 (#1813) Reed
[c10ef33] Only enable external sourcemaps for prod builds (#1811) Sebastian Klingler
[67e9c2d] [PAY-479] Check recipient wallet SOL before send (#1779) Reed
@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.

None yet

3 participants