[C-1461] Implement license-type/isrc upload flow #2248
Conversation
dylanjeffers
requested review from
a team and
amendelsohn
and removed request for
a team
|
Preview this change https://demo.audius.co/dj-c-1461-license-type-upload-flow |
| @@ -10,23 +10,23 @@ import { makeStyles } from 'app/styles' | |||
| // Note, offset is the inner padding of the container div | |||
| const offset = 3 | |||
|
|
|||
| export type Option = { | |||
| key: string | |||
| export type Option<Value> = { | |||
There was a problem hiding this comment.
Brain snagged on this one for a sec. Reminded me of https://en.wikipedia.org/wiki/Option_type, though the name collision is probably fine here
| @@ -99,7 +99,9 @@ const useStyles = makeStyles(({ palette, typography, spacing }) => ({ | |||
| } | |||
| })) | |||
|
|
|||
| export const SegmentedControl = (props: SegmentedControlProps) => { | |||
| export const SegmentedControl = <Value,>( | |||
There was a problem hiding this comment.
It's this annoying issue with generic types and anonymous functions, we need it sadly
| @@ -242,6 +243,7 @@ export const TextInput = forwardRef<RNTextInput, TextInputProps>( | |||
| value={value} | |||
| onFocus={handleFocus} | |||
| onBlur={handleBlur} | |||
| placeholder={label && !isFocused ? undefined : placeholder} | |||
There was a problem hiding this comment.
Why do we need !isFocused? Does RNTextInput not hide the placeholder while you're typing by default?
There was a problem hiding this comment.
Good q so for cases when there is a label, that's in the placeholder spot till you click into it, the label animates up, and then a placeholder text can appear. Otherwise there is a collision between the two
| ) : null | ||
| }, [allowAttribution, commercialUse, derivativeWorks, styles, neutral]) | ||
|
|
||
| console.log('license', license) |
| @@ -0,0 +1,148 @@ | |||
| import { useCallback, useMemo, useState } from 'react' | |||
There was a problem hiding this comment.
Assuming #2246 and this one have the same changes on purpose. This builds on that?
There was a problem hiding this comment.
Yeah there were merge conflicts otherwise, but after that one is merged and this is rebased, it should go away
| > | ||
| <View style={styles.content}> | ||
| <View style={styles.licenseSection}> | ||
| <Text {...labelProps}>{messages.allowAttribution}?</Text> |
There was a problem hiding this comment.
Is there a reason to keep the ? outside messages? That might not be appropriate for all localizations
There was a problem hiding this comment.
Great callout, I was using messages in multiple spots so I should probably just duplicate them
| derivativeWorks: Nullable<boolean> | ||
| ) => { | ||
| if (!allowAttribution) return null | ||
| const icons: [ComponentType<SvgProps>, string][] = [ |
There was a problem hiding this comment.
realized i could have made this more clear with
const icons: [Icon: ComponentType<SvgProps>, key: string][] = [
updated!
so apparently this is not possible with react-native and ios, since ios defines it's own textBreakStrategy? there is an android specific option though looks like: https://reactnative.dev/docs/text#textbreakstrategy-android |
[ad230d1] v1.4.0 (#2242) Sebastian Klingler [dc11096] Stop propagation on repost button click in tracks tables (#2249) Kyle Shanks [5964998] Update push notification types, update notif navigation hook, and remove userInteraction check in onNotification method (#2247) Kyle Shanks [decabd2] [C-1461] Implement license-type/isrc upload flow (#2248) Dylan Jeffers [9e84248] [C-1452] Implement release-date field (#2246) Dylan Jeffers [8a0c7a8] Fix race condition checking stream_mp3 feature flag (#2244) Andrew Mendelsohn [36d53b0] Don't delete sourcemaps on sync to s3 (#2243) Sebastian Klingler
Description
RPReplay_Final1668131810.mov