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

[PAY-1618] Update DogEar rendering and fix spacing #3775

Merged
merged 7 commits into from
Jul 21, 2023

Conversation

schottra
Copy link
Contributor

Description

DogEars were rendering with a visible gap around the outside (the border of the container beneath them). The fix for this turned out to be a little trickier than expected, partially due to the way the component was implemented.

The original implementation was done with an 80x80px square linear gradient, rotated 45 degrees and then clipped by the parent container's border box to achieve the desired shape. While this works, it had two disadvantages:

  • It requires overflow:hidden to be set on the parent. When using this overflow setting, no child is allowed to exceed the border box. And that means the DogEar cannot overlap the border, and the border will be visible around the outside.
  • It was not in sync with the vectors being used in the Figma designs

I brought in the new SVG, which uses a path to draw the rounded edge and has the correct gradients built in. Overflow behavior was removed from the parent container, and logic was added to allow parents to position the DogEar with an offset to account for their own borders.

Dragons

This did involve some changes to the Tile component on desktop, which is used as a base for components other than the TrackTile. But the changes are minimal and I did a verification pass to make sure none of them looked strange without the overflow behavior.

How Has This Been Tested?

Tested locally in Chrome (web and mobile web emulator), on iOS Simulator and physical Android device.

How will this change be monitored?

Will verify as part of RC process and revert if necessary.

Feature Flags

N/A

Screenshots

Web Track Tile
Before
Screenshot 2023-07-20 at 10 57 34 AM

After
Screenshot 2023-07-20 at 10 50 16 AM


Mobile Web Track Tile
Before
Screenshot 2023-07-20 at 10 56 39 AM

After
Screenshot 2023-07-20 at 10 50 54 AM


Mobile Track Tile
Before
Screenshot 2023-07-20 at 10 59 00 AM

After
Screenshot 2023-07-20 at 10 06 02 AM


Web Track Details
Before
Screenshot 2023-07-20 at 10 57 44 AM

After
Screenshot 2023-07-20 at 11 04 06 AM


Mobile Web Track Details
Before
Screenshot 2023-07-20 at 10 57 02 AM

After
Screenshot 2023-07-20 at 11 01 04 AM


Mobile Track Details
Before
Screenshot 2023-07-20 at 10 58 46 AM

After
Screenshot 2023-07-20 at 10 52 32 AM

@@ -1,61 +1,49 @@
.artistPick {
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 don't know why this class was named artistPick (probably the dogEar was originally just for that?), but it was clearly the "container" 😄


.borderOffset {
position: absolute;
top: calc(-1 * var(--border-width));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open to feedback on this pattern. My goal with this PR was to remove knowledge of the parent from DogEar. The place where we decide how far to offset a component to cover the border is the place that defines said border!

Copy link
Contributor

Choose a reason for hiding this comment

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

if someone overwrites borderWidth later in a different style, that won't get picked up here right? They'd have to set var(--border-width) instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you set a class or inline style on any component using this class, and within that class/style set --border-width: value, this will pick up that value and use it. This is also inheriting the value from the nearest parent that has it set (in this case .root), so you can set the value on that instead.

If you were trying to do it from outside this module or the corresponding component, it might be a little trickier. But I think the idea is that this stays contained to only this module.

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/pay-1618-fix-dog-ears

left: 0,
width: spacing(12),
height: spacing(12),
shadowColor: '#000',
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: 'black'?

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 was following another pattern for setting shadow, and also this value matches what is shown in Figma. Why the preference for the named color?

<Icon fill={staticWhite} height={16} width={16} style={styles.icon} />
<View style={[styles.container, borderOffsetStyle, style]}>
<DogEarRectangle fill={colors[0]} style={styles.rectangle} />
<Icon width={16} height={16} fill={staticWhite} style={styles.icon} />
Copy link
Contributor

Choose a reason for hiding this comment

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

spacing(4) instead of 16

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct! I thought you could only use that function in makeStyles context and I was mistaken.

}

/* Note: dogEarRectangle has multiple `path` elements, but we only want to fill one of them.
That path uses `fill='currentColor'`, allowing us to set `color` here instead of `fill` */
Copy link
Contributor

Choose a reason for hiding this comment

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

nice.


.borderOffset {
position: absolute;
top: calc(-1 * var(--border-width));
Copy link
Contributor

Choose a reason for hiding this comment

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

if someone overwrites borderWidth later in a different style, that won't get picked up here right? They'd have to set var(--border-width) instead?

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/pay-1618-fix-dog-ears

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/pay-1618-fix-dog-ears

@schottra schottra merged commit 3e24539 into main Jul 21, 2023
2 checks passed
@schottra schottra deleted the pay-1618-fix-dog-ears branch July 21, 2023 14:33
dylanjeffers added a commit that referenced this pull request Jul 24, 2023
[72ec48f] Upgrade to 1.5.33 Dylan Jeffers
[9ec1f54] Update to reroute to trending when deleting a playlist if the playlist is viewed (#3789) Kyle Shanks
[5cbc230] [C-2877] Address suggested-tracks qa #2 (#3787) Dylan Jeffers
[4ad7987] [PAY-1652] Update LockedContentDrawer on mobile (#3786) Reed
[5e2337c] Fix mobile premcontent locked badge colors (#3783) Reed
[19cca4c] Change mobile drawer background to white (#3784) Reed
[05b7d1f] [PAY-1635] Always show share button for track owners on hidden tracks (#3782) Randy Schott
[ca66d4d] [PAY-1575] Always show premium DogEars on track details page (#3781) Randy Schott
[46f41a8] [PAY-1620] Fix disabled play button for unlocked gated tracks on Mobile Web (#3779) Randy Schott
[44666b2] [PAY-1615] Fix display of "Artist Pick" text (#3780) Randy Schott
[3e24539] [PAY-1618] Update DogEar rendering and fix spacing (#3775) Randy Schott
[bb28a09] [C-2866] Undo secondary button style changes (#3777) Randy Schott
[699f8f6] [PAY-1634] Fix hidden track tile + add share button (#3765) Reed
[1d95e73] Add support for UDSC Mint in Client (#3776) Marcus Pasell
[79d85f1] [PAY-1638] Fix iconLockUnlocked icon (#3766) Reed
[b63107b] [C-2872] Fix image retries (#3773) Dylan Jeffers
[b99b93a] Add fallback url to embed (#3772) Raymond Jacobson
[5adb25b] Remove usages of CN /health_check/verbose (#3769) Theo Ilie
[62a0c72] [C-2846] Suggested Tracks QA (#3771) Dylan Jeffers
[9c87fb2] [C-2861] Replace private with hidden (#3768) Dylan Jeffers
[87f7f35] [C-2650] Add playlist-library sanitization hook (#3767) Dylan Jeffers
[8ba6d04] Add Dapp store publishing in CI (#3747) Raymond Jacobson
[9324a06] [C-2842] Improve playlist image generation (#3762) Dylan Jeffers
[9c5a412] Remove unused notification announcement page (#3739) Reed
[5023cef] [C-2853] Fix deletedCount calculation (#3763) Andrew Mendelsohn
[b627216] [PAY-1639] Fix mobile hidden track dog ear (#3764) Reed
[40ea57e] [C-2856] Fix track/collection artwork field (#3760) Dylan Jeffers
[366e4df] [C-2858] Fix phantom signing (#3761) Raymond Jacobson
[8a7cbc7] [PAY-1621] Reverse order of mobile lineup tile stat icon and number (#3759) Reed
[9206ab0] [PAY-1612] Change mobile hidden track header color (#3758) Reed
[745ff35] Use Text component in mobile DMs (#3749) Reed
[1afc426] [PAY-1607] Fix mobile prem content track tile layout (#3757) Reed
[07f0f6c] [PAY-1606] Updates gated content experience  (#3754) Randy Schott
[a82e296] v1.5.32 Dylan Jeffers
dharit-tan pushed a commit that referenced this pull request Jul 24, 2023
[72ec48f] Upgrade to 1.5.33 Dylan Jeffers
[9ec1f54] Update to reroute to trending when deleting a playlist if the playlist is viewed (#3789) Kyle Shanks
[5cbc230] [C-2877] Address suggested-tracks qa #2 (#3787) Dylan Jeffers
[4ad7987] [PAY-1652] Update LockedContentDrawer on mobile (#3786) Reed
[5e2337c] Fix mobile premcontent locked badge colors (#3783) Reed
[19cca4c] Change mobile drawer background to white (#3784) Reed
[05b7d1f] [PAY-1635] Always show share button for track owners on hidden tracks (#3782) Randy Schott
[ca66d4d] [PAY-1575] Always show premium DogEars on track details page (#3781) Randy Schott
[46f41a8] [PAY-1620] Fix disabled play button for unlocked gated tracks on Mobile Web (#3779) Randy Schott
[44666b2] [PAY-1615] Fix display of "Artist Pick" text (#3780) Randy Schott
[3e24539] [PAY-1618] Update DogEar rendering and fix spacing (#3775) Randy Schott
[bb28a09] [C-2866] Undo secondary button style changes (#3777) Randy Schott
[699f8f6] [PAY-1634] Fix hidden track tile + add share button (#3765) Reed
[1d95e73] Add support for UDSC Mint in Client (#3776) Marcus Pasell
[79d85f1] [PAY-1638] Fix iconLockUnlocked icon (#3766) Reed
[b63107b] [C-2872] Fix image retries (#3773) Dylan Jeffers
[b99b93a] Add fallback url to embed (#3772) Raymond Jacobson
[5adb25b] Remove usages of CN /health_check/verbose (#3769) Theo Ilie
[62a0c72] [C-2846] Suggested Tracks QA (#3771) Dylan Jeffers
[9c87fb2] [C-2861] Replace private with hidden (#3768) Dylan Jeffers
[87f7f35] [C-2650] Add playlist-library sanitization hook (#3767) Dylan Jeffers
[8ba6d04] Add Dapp store publishing in CI (#3747) Raymond Jacobson
[9324a06] [C-2842] Improve playlist image generation (#3762) Dylan Jeffers
[9c5a412] Remove unused notification announcement page (#3739) Reed
[5023cef] [C-2853] Fix deletedCount calculation (#3763) Andrew Mendelsohn
[b627216] [PAY-1639] Fix mobile hidden track dog ear (#3764) Reed
[40ea57e] [C-2856] Fix track/collection artwork field (#3760) Dylan Jeffers
[366e4df] [C-2858] Fix phantom signing (#3761) Raymond Jacobson
[8a7cbc7] [PAY-1621] Reverse order of mobile lineup tile stat icon and number (#3759) Reed
[9206ab0] [PAY-1612] Change mobile hidden track header color (#3758) Reed
[745ff35] Use Text component in mobile DMs (#3749) Reed
[1afc426] [PAY-1607] Fix mobile prem content track tile layout (#3757) Reed
[07f0f6c] [PAY-1606] Updates gated content experience  (#3754) Randy Schott
[a82e296] v1.5.32 Dylan Jeffers
@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