Add pill radio button by rickyrombo · Pull Request #1745 · AudiusProject/audius-client

rickyrombo · 2022-08-18T19:21:18Z

Description

Adds a new component that's an accessible "Pill" radio button

Currently localized to the BuyAudoModal but can easily extract to stems if we need to use elsewhere.

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.

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 · 2022-08-18T19:29:19Z

Preview this change https://demo.audius.co/mjp-pill-radio-button

dharit-tan · 2022-08-18T19:43:58Z

+  justify-content: center;
+  align-items: center;
+
+  /* height: 45px; */


nit: remove comment?

dylanjeffers

This looks great! a few callouts would love to hear your thoughts:

Do we want to define a radio-group component?
Are there use cases where the pill is not a radio button and just a button?
Do we want :focus styles, like when someone tabs to the button?
Do we want to move this to stems at some point?
Do we want to have accessibilty options to have a better label, instead of just a number? so like it would read 5 audio, 10 audio etc? there are a few options here if we wanna go there

rickyrombo · 2022-08-18T23:16:56Z

This looks great! a few callouts would love to hear your thoughts:

Do we want to define a radio-group component?

Are there use cases where the pill is not a radio button and just a button?

Do we want :focus styles, like when someone tabs to the button?

Do we want to move this to stems at some point?

Do we want to have accessibilty options to have a better label, instead of just a number? so like it would read 5 audio, 10 audio etc? there are a few options here if we wanna go there

My take: yes to all of this!

On it!
This already exists in Stems
This sounds like a Julian question, but I think that would be good! I'll add to my list of Julian Q's
I can move it to stems now if you'd like. I don't see anywhere else this is used currently (the others are all buttons).
Would love to pick your brain here - would that not be simply specifying the aria-label on the inputProps?

…into mjp-pill-radio-button

dylanjeffers · 2022-08-18T23:55:16Z

This looks great! a few callouts would love to hear your thoughts:

Do we want to define a radio-group component?

Are there use cases where the pill is not a radio button and just a button?

Do we want :focus styles, like when someone tabs to the button?

Do we want to move this to stems at some point?

Do we want to have accessibilty options to have a better label, instead of just a number? so like it would read 5 audio, 10 audio etc? there are a few options here if we wanna go there

My take: yes to all of this!

On it!

This already exists in Stems

This sounds like a Julian question, but I think that would be good! I'll add to my list of Julian Q's

I can move it to stems now if you'd like. I don't see anywhere else this is used currently (the others are all buttons).

Would love to pick your brain here - would that not be simply specifying the aria-label on the inputProps?

1-3 sg
4. yeahh... since one already exists in stems maybe we hold off to see if this would be a general pattern in a few places?
5. yeah i think just passing aria-label could be good, but since you already have a label element thought i think it is best to update that one. not sure what is absolute best way, but i think using a visually hidden span might be the move there

…into mjp-pill-radio-button

dylanjeffers

looks absolutely perfect

dylanjeffers · 2022-08-22T17:27:41Z

+    [setValueState, onChange]
+  )
+  return (
+    <RadioGroupContext.Provider value={{ name, onChange: handleChange, value }}>


love that you added the context and everything

dylanjeffers · 2022-08-22T17:27:59Z

+    name,
+    onChange,
+    children,
+    value: valueProp,


also love that we are using the valueProp rename + the useControlled

dylanjeffers · 2022-08-22T17:28:14Z

+    defaultValue,
+    ...divProps
+  } = props
+  const [value, setValueState] = useControlled({


personally like spaces between large hook blocks :)

dylanjeffers · 2022-08-22T17:29:32Z

+    }
+  }
+
+  const handleChange = useCallback(


awesome stuff

[ccf1ca9] [C-906] Migrate queue sagas and hls-utils to common (#1805) Dylan Jeffers [fe441af] Consume new Text Button in collapsible content (#1790) Marcus Pasell [2ee3b2d] Add error UI to InProgress page of BuyAudio modal (#1802) Marcus Pasell [139e50e] Add TextButton story (#1803) Marcus Pasell [a4d26cf] Add all the pages to the BuyAudioModal (#1800) Marcus Pasell [15dfb72] Update package.json (#1801) Raymond Jacobson [284431d] [C-817] Deep link /settings on mobile (#1796) Raymond Jacobson [2b7ca95] [PAY-44] Rename getAssociatedTokenAccountInfo and getUserBank (#1794) Reed [ff07bb4] [PAY-580] Add Success Page to BuyAudio Modal (#1791) Marcus Pasell [51269bb] [C-901] Move audioPlayer to saga context (#1797) Dylan Jeffers [dfc6f08] [C-900] Move queue, lineup, feed-page sagas to common (#1795) Dylan Jeffers [4f25517] [C-897] Add explore service to saga context (#1793) Dylan Jeffers [8cc3a29] Move sagaHelpers to common (#1792) Sebastian Klingler [8ee3aa8] Add Text Button type to Stems (#1789) Marcus Pasell [ff4f913] [PAY-552] Change playllenge challenge button text (#1787) Reed [60ed26a] Fix bug with renamed component (#1788) Marcus Pasell [718f35a] Dynamically resize ModalContentPages (#1785) Marcus Pasell [4903c9f] [PAY-577] Add "in progress" page to Buy Audio Modal (#1783) Marcus Pasell [72fc4fd] Migrate queue selectors to common (#1786) Dylan Jeffers [5f3e46e] [C-871] Migrate player state to common (#1784) Dylan Jeffers [8d77abf] [PAY-476] Display error when send to wallet fails (#1780) Reed [6adbdda] [PAY-575] [PAY-574] [PAY-573] [PAY-572] [PAY-581] Coinbase Pay Button and Rewards Page Fixes (#1778) Marcus Pasell [56c3f39] [PAY-458] Buy Audio Modal Input Page (#1777) Marcus Pasell [172d6c4] Cleans up code for Challenge Twitter icon fill color (#1781) Reed [bd1a329] Fix order playlist to use updated playlist (#1774) Isaac Solo [1f178bc] [C-887] Host sourcemaps on s3, fixes cloudflare deploy (#1776) Sebastian Klingler [0551dec] [PAY-571] Move Jupiter out of bundle, attach BuyAudio sagas (#1772) Marcus Pasell [db9cd4a] [C-890] Polyfill TextEncoder, fixes builds (#1775) Dylan Jeffers [de8c73a] Update @coinbase/cbpay-js to v1.2.0 and @solana/web3.js to 1.53.0 (#1773) Marcus Pasell [bdec4b7] Add entity manager address to configInternalWeb3 (#1763) nicoback2 [d3fae31] Wire up initial BuyAudioModal (#1770) Marcus Pasell [fc5e484] Add back modal hook functionality (#1769) Marcus Pasell [ae616cc] Fix @audius/common external warnings, model imports, lint (#1767) Dylan Jeffers [2a121e4] Add entity manager address for prod env (#1766) Isaac Solo [b0c00f1] [C-875] Remove blocklist (#1764) Dylan Jeffers [859df1f] [RN Reloaded] Migrate web common/store and common/hooks to @audius/common (#1675) nicoback2 [bdd9e03] [PAY-546] Add memo instruction to final onramp transfer, fix rent min problem (#1731) Marcus Pasell [635a6ed] [PAY-431] Slide in supporters after loading (#1720) Reed [2af212a] [PAY-438] Fix Twitter icon fill color in matrix mode (#1760) Reed [bd925e1] Add pill radio button (#1745) Marcus Pasell [0392854] Remove eager discprov logs (#1762) Dylan Jeffers [b6733ef] Update @audius/sdk to 0.0.38 (#1757) Isaac Solo [a5e941e] [C-862] Add env to saga context, fix eagerDiscprov in native (#1753) Dylan Jeffers [873ed99] Remove /macro usage for typed-redux-saga (#1751) (#1755) Dylan Jeffers [ca202fe] Update playlist actions to use entity manager (#1674) Isaac Solo [48ae001] Fix margin issue (#1752) Raymond Jacobson

Add pill radio button

98c85b2

rickyrombo assigned dharit-tan, piazzatron, dylanjeffers and sddioulde Aug 18, 2022

pull-request-size Bot added the size/M label Aug 18, 2022

dharit-tan approved these changes Aug 18, 2022

View reviewed changes

dylanjeffers approved these changes Aug 18, 2022

View reviewed changes

rickyrombo added 2 commits August 18, 2022 16:45

Merge branch 'main' of https://github.com/AudiusProject/audius-client …

e452a32

…into mjp-pill-radio-button

remove comment

8cc2f4a

Merge branch 'main' of https://github.com/AudiusProject/audius-client …

aaa4848

…into mjp-pill-radio-button

piazzatron approved these changes Aug 19, 2022

View reviewed changes

rickyrombo added 2 commits August 18, 2022 19:26

Move to stems, make radio group

6cedfcb

Add focus state

17396ba

pull-request-size Bot added size/L and removed size/M labels Aug 19, 2022

rickyrombo added 2 commits August 19, 2022 18:51

Make radio group controlled

63f5d31

Add license notice

0d76e35

rickyrombo requested a review from dylanjeffers August 22, 2022 17:21

dylanjeffers approved these changes Aug 22, 2022

View reviewed changes

rickyrombo merged commit bd925e1 into main Aug 22, 2022

rickyrombo deleted the mjp-pill-radio-button branch August 22, 2022 17:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pill radio button#1745

Add pill radio button#1745
rickyrombo merged 8 commits into
mainfrom
mjp-pill-radio-button

rickyrombo commented Aug 18, 2022

Uh oh!

audius-infra commented Aug 18, 2022

Uh oh!

dharit-tan Aug 18, 2022

Uh oh!

dylanjeffers left a comment •

edited

Loading

Uh oh!

rickyrombo commented Aug 18, 2022

Uh oh!

dylanjeffers commented Aug 18, 2022 •

edited

Loading

Uh oh!

dylanjeffers left a comment

Uh oh!

dylanjeffers Aug 22, 2022

Uh oh!

dylanjeffers Aug 22, 2022

Uh oh!

dylanjeffers Aug 22, 2022

Uh oh!

dylanjeffers Aug 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

rickyrombo commented Aug 18, 2022

Description

Dragons

How Has This Been Tested?

How will this change be monitored?

Feature Flags

Uh oh!

audius-infra commented Aug 18, 2022

Uh oh!

dharit-tan Aug 18, 2022

Choose a reason for hiding this comment

Uh oh!

dylanjeffers left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rickyrombo commented Aug 18, 2022

Uh oh!

dylanjeffers commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dylanjeffers left a comment

Choose a reason for hiding this comment

Uh oh!

dylanjeffers Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

dylanjeffers Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

dylanjeffers Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

dylanjeffers Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dylanjeffers left a comment •

edited

Loading

dylanjeffers commented Aug 18, 2022 •

edited

Loading