-
Notifications
You must be signed in to change notification settings - Fork 43
Conversation
Preview this change https://demo.audius.co/rt-dog-ear-common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work!
premiumConditions?: Nullable<PremiumConditions> | ||
} | ||
|
||
export const getDogEarType = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we also have dog ear for collections, can we specify this to either handle both, or just say this is track related?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this already handles both - correct me if I'm wrong but I grepped DogEar
in web
and I believe I've changed every instance. Only saw one collection-related instance on CollectionPage
that was only used for HIDDEN
so that case is covered.
Maybe I should just change the file name to dogEarUtils.ts
.
@@ -263,23 +256,12 @@ export const DetailsTile = ({ | |||
}, [onPressPlay]) | |||
|
|||
const renderDogEar = () => { | |||
const showPremiumDogEar = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really smart not having to duplicate this everywhere!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome.
I have a PR ready for mobile web, so I'll verify it there when I merge in this change.
Description
dogEarType
logic was getting hairy and also we were doing the same calculations in every component that used aDogEar
. Moved to a util fn incommon
and updated all the callsites.getDogEarType
should now return null if there's not an exact match on the conditions, which also makes it possible to delete all theshowPremiumContent
vars too.Dragons
How Has This Been Tested?
Local ios + web stage
There are probably some cases that I've missed but I think it's ok.
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.