useHarmonyField #7514

rickyrombo · 2024-02-08T01:56:54Z

Description

Creates a hook to handle the state of formik fields for harmony components. Is agnostic so reduces code duplication across web/mobile and along text field/password field

How Has This Been Tested?

Made a test page and tested the hook with various inputs on web including:

HarmonyTextInput
HarmonyPasswordInput
SelectablePill (single checkbox, checkboxes)
RadioGroup w/ RadioInput
SegmentedControl (well, a new version)

Also tested mobile with ChangePasswordScreen updates.

Planning to do the migration of Field components to this hook in a later PR, as well as add tests.

Also includes a fix to SelectablePill to include styling when checked but isSelected is unset

audius-infra · 2024-02-08T02:06:58Z

Preview this change https://demo.audius.co/mjp-useHarmonyField

packages/web/src/components/change-password/ChangePasswordModal.tsx

gitguardian · 2024-02-14T09:50:28Z

⚠️ GitGuardian has uncovered 4 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
9412812	Triggered	Generic Password	`5d54b1b`	packages/discovery-provider/ddl/local-test.sh	View secret
9412812	Triggered	Generic Password	`5d54b1b`	packages/discovery-provider/ddl/docker-compose.yml	View secret
9349019	Triggered	Generic High Entropy Secret	`02c4ef1`	dev-tools/config.json	View secret
9368958	Triggered	Generic Password	`02c4ef1`	dev-tools/compose/docker-compose.ddex.yml	View secret

🛠 Guidelines to remediate hardcoded secrets

Understand the implications of revoking this secret by investigating where it is used in your code.
Replace and store your secrets safely. Learn here the best practices.
Revoke and rotate these secrets.
If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

following these best practices for managing and storing secrets including API keys and other credentials
install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!}

audius-infra · 2024-02-14T10:03:55Z

Preview this change https://demo.audius.co/mjp-useHarmonyField

audius-infra · 2024-02-15T01:36:14Z

Preview this change https://demo.audius.co/mjp-useHarmonyField

audius-infra · 2024-02-16T01:57:30Z

Preview this change https://demo.audius.co/mjp-useHarmonyField

DejayJD · 2024-02-16T21:24:11Z

packages/common/src/hooks/useHarmonyField.ts

+  /** Function to transform the input value on blur, eg. a function to trim whitespace */
+  transformValueOnBlur?: (value: string) => string
+  /**
+   * Delays onChange validation errors from showing. Useful for improving the


this is probably not the highest priority (I cant even think of an exact scenario for it); but I would expect this debounce prop to behave the same for onBlur validations as well

Also side note; would this occur after transforms have occurred?

I actually initially had it debounce validation for onBlur too anyway but I figure it might make sense to not delay that one unnecessarily.. but maybe this is causing a race condition 🤔 good callout

But yeah the debouncing is only for the validation, so yeah it's after the other transforms.

DejayJD · 2024-02-16T21:27:19Z

packages/common/src/hooks/useHarmonyField.ts

+ * }
+ */
+export const useHarmonyField = <Value = any>(
+  propsOrFieldName: string | UseHarmonyFieldProps<Value>


nit: seems a bit convoluted to attempt to squeeze in string as an option here. I get why it would be nice to just do useHarmonyField("email") but I think it's probably just as easy to do useHarmonyField({name: "email"}) and that way its a bit more standardized and simpler implementation

yeah, given we're likely doing this in field components anyway it's somewhat moot. I did want to keep parity with the formik hook though, not sure if that's worth it

DejayJD · 2024-02-16T21:32:04Z

packages/common/src/hooks/useHarmonyField.ts

+  } = useFormikContext()
+
+  // Tracks whether the user is actively editing the field
+  const [isChanging, setIsChanging] = useState(false)


love this. It baffles me that Formik didn't really come with this concept built in 💀

DejayJD · 2024-02-16T21:35:57Z

packages/common/src/hooks/useHarmonyField.ts

+      const maybePromise = setValue(value, false) as Promise<any> | void
+      if (maybePromise) {


whats the scenario when this is a promise vs not?

it's actually always a promise since we're not validating, but figure it's best to be a bit defensive anyway: https://github.com/jaredpalmer/formik/blob/0f960aaeeb0bdbef8312b5107cd3374884a0e62b/packages/formik/src/Formik.tsx#L583-L598

surprisingly the types lie

Fair, thanks!

DejayJD · 2024-02-16T21:48:10Z

packages/common/src/hooks/useHarmonyField.ts

+      onChange,
+      onBlur
+    },
+    { ...meta, error: hasError ? error : undefined },


I can think of scenarios where we may want to use different error logic than the hasError logic built in here (i.e. ignoring touched or isChanging), so "squashing" the old error here may be a bit problematic.
The two scenarios that come to my mind right now are on the handle page we validate on mount (touched is false), and we also validate while the user is typing (isChanging would be false since it hasn't been blurred yet)

Maybe we can include the original formik error still somehow?

Ooh excellent callout! I had assumed we never want to show errors when the user hasn't had a chance to type anything yet - didn't consider prefilled forms.

Will think a bit more on this. Off the cuff, I'm down to make a separate "hasError" meta field prop, but what about adding a specific check for the validateOnMount case? And for the other case, I think we'll want to debounce that anyway so setting debounceIdleMs to something nonzero should solve? And if any other scenarios come up, we can call useField from Formik and override the hook via the Field component's props?

DejayJD

LGTM! Excited to use it. Overall looks very clean and looks like a much needed standard. I can also tell you've been super thoughtful with some of the edge cases, great job on that.
I'm definitely gonna play around with it for sign up fixes next week; might have more feedback after working with it for a bit.

audius-infra · 2024-03-01T00:14:29Z

Preview this change https://demo.audius.co/mjp-useHarmonyField

dylanjeffers

Let's def merge this and will be good to have this improved implementation of useField to compare and swap in. Your command of the complexity here is super impressive

dylanjeffers · 2024-03-01T01:06:49Z

packages/common/src/hooks/useHarmonyField.ts

+      ? validateFormOnChange
+      : validateFieldOnChange
+
+  // Only show an error state if:


Love all the documentation, and how it's all document and defined in one hook

dylanjeffers · 2024-03-01T01:07:09Z

packages/common/src/hooks/useHarmonyField.ts

+  FieldMetaProps<Value>,
+  FieldHelperProps<Value>
+] => {
+  const props =


dylanjeffers · 2024-03-01T01:08:39Z

packages/common/src/hooks/useHarmonyField.ts

+    // We'll validate ourselves - pass false for validation
+    setTouched(true, false)
+    // Cast back to promise (the types are wrong)
+    const maybePromise = validateField(name) as Promise<any> | void


So does this prevent form level validation from firing for this field, or is there a case that we get double validation?

Good callout, sent me back into the rabbit hole a bit 😂

What I found is basically everywhere we pass in a Zod schema as the validationSchema anyway, which means that there's really no difference between "field level" vs "form level" validation - the whole schema is validated no matter what. The goal of what I'm doing here is basically allowing us to wait for validation instead of doing it ahead of time. I just realized though that unlike the field.onBlur callback, setTouched returns the validation promise, so we don't need to split this out into two calls!

I'll make that change...!

dylanjeffers · 2024-03-01T01:09:34Z

packages/common/src/hooks/useHarmonyField.ts

+    debouncedIdleMs
+  )
+
+  const onChangeText = useCallback(


Feels a little strange to have onChangeText defined generally for all field types?

Edit: I see now that we do the check later on :)

dylanjeffers · 2024-03-01T01:12:34Z

packages/harmony/src/components/input/SelectablePill/SelectablePill.tsx

@@ -35,6 +26,10 @@ export const SelectablePill = (props: SelectablePillProps) => {
  } = props

  const { disabled, type } = other
+  const isSelected =


[73c35e5] Bump mobile app version for full-app release (#7770) Dylan Jeffers [fa9c7ce] [PAY-2490] Delete backfill txs tables (#7768) Reed [acf5840] Fix reset-password-modal icon + ui (#7769) Dylan Jeffers [cb9c022] [C-3929] Fix select-artist genres (#7765) Dylan Jeffers [709ff70] Fix reset password (#7767) Marcus Pasell [1c1474e] [C-3708] Fix wallet text clipping issue in withdrawals table (#7762) Kyle Shanks [7f4b187] Add an isNativeMobile prop to amplitude (#7766) JD Francis [4f6972f] [PAY-2527] Return `playlists_containing_track` from track apis (#7764) Andrew Mendelsohn [dbc6dc1] Fix download buttons erroneously displaying (#7763) Reed [8cf2464] [PAY-2531] Grant access to stream track if user has purchased album (#7753) Reed [762a9ac] [C-3926] Add error helper text to new sign in password field (#7761) Kyle Shanks [6d6117f] [C-3885] Migrate to harmony text-link (#7746) Dylan Jeffers [82dd8c5] Update dapp-store build artifacts audius-infra [1c554c6] [C-3924] Fix misc font/type issues on purchase flow (#7760) Raymond Jacobson [b695203] [C-3923] Remove double badges on purchase flow (#7759) Raymond Jacobson [e1dd6e0] [PAY-2488] Remove stripe option from purchase flow (#7755) Raymond Jacobson [4931acb] Bump version to 0.6.50 audius-infra [c8eaa02] [C-2928] Add auth request signing middleware to sdk (#7724) Raymond Jacobson [0d2d3b1] Add verifier checks to discovery relay (#7756) Raymond Jacobson [c3f0a99] useHarmonyField (#7514) Marcus Pasell [ce226e7] Publish harmony to npm (#7751) Sebastian Klingler [d5c0c98] [C-3897] Fix ai-attribution dropdown (#7754) Dylan Jeffers [26f9c7d] [PAY-2533] Index album purchases (#7735) Andrew Mendelsohn [375f5b5] [C-3888] Add header-shadow to sign-up-screen (#7750) Dylan Jeffers [dbbbc88] @audius/sdk: v3.0.35 audius-infra [5125501] [DRVL-16] - Documentation Site Updates (#7699) Sam Gutentag [2738765] Use turbo for sdk release (#7748) Sebastian Klingler [23072cd] [C-3918] Fix wallet and dev-apps icons (#7749) Dylan Jeffers [a4bbe1c] Update stem limit to 20 (#7745) Raymond Jacobson [f4863c6] Fix social sign ups redirecting unintentionally (#7744) JD Francis [557142b] Disable play button if all collection tracks deleted (#7743) Saliou Diallo [5f28d5d] Add others to skip otp (#7741) Raymond Jacobson [b33bc5b] Add download gated column (#7742) Saliou Diallo [9f0f84e] Bump version to 0.6.49 audius-infra [c2a9c46] Add ddex_app field for ddex tracks/playlists to discovery (#7734) Michelle Brier [c932495] [PROTO-1686] Setup DDEX choreography infra and related improvements (#7739) Theo Ilie [aec57f9] Misc sign up social fixes (#7737) JD Francis [682a384] [ONC-35] Fix crash on android due to react native importing images as numbers (#7738) Marcus Pasell [273be01] Misc Sign up fixes (spacing & page swipe animation) (#7736) JD Francis [cb39083] [ONC-34] Fix bug in mobile suggested collection tracks (#7731) Saliou Diallo [8db7400] [C-3866] Update placement of theme reset in mobile sign out to make colors display properly (#7728) Kyle Shanks [0d15ae7] [C-3898] Fix faded gray screen android on sign up (#7730) JD Francis [a0d314b] Bump version to 0.6.48 audius-infra [f43e956] [PROTO-1513] Add e2e ddex test (#7733) Theo Ilie [f5ad4c3] Fix harmony ssr (#7732) Dylan Jeffers [06af9d2] [C-3909] Fix upload track challenge copy (#7729) Saliou Diallo [8f73c19] [C-3899] Fix android able to go back on signup (#7727) JD Francis [d04b4bc] [PAY-2529] Index album stream_conditions (#7720) Reed [8966f88] [QA-779][QA-704] Fix some UI issues (#7725) Saliou Diallo [d29c83f] Misc Sign Up QA Fixes (#7722) JD Francis [90b098a] Discovery placement hosts (#7718) Steve Perkins [9674780] Bump version to 0.6.47 audius-infra [398db27] PROTO-1574: sepolia (#7698) alecsavvy [40583e6] Improve search analytics (#7696) Isaac Solo [3e1b544] Add env linter (#7697) Raymond Jacobson [5a3498c] [DVRL-11] Move client history requests to sdk (#7679) Raymond Jacobson [beb054b] Remove all usage of PROTOCOL_DIR env var (#7714) Danny [6230eca] [PAY-2528] is_stream_gated + stream_conditions on Playlist (#7717) Reed [4136985] [PAY-2524, PAY-2525, PAY-2526] Add collections_containing_track field to tracks (#7708) Andrew Mendelsohn [f25cc23] Fix the same typo in another spot (#7716) Theo Ilie [f51ccd8] [C-3858] Additional padding on select artists screen (#7715) JD Francis [9ff7262] [C-3858] Social sign on loading screen adjustments (#7710) JD Francis [4c3a35c] Fix typo breaking audius-compose push (#7713) Theo Ilie [74facea] Bump mobile versions for prod release (#7712) Dylan Jeffers [392bb21] Bump version to 0.6.46 audius-infra

rickyrombo added 8 commits February 7, 2024 14:52

Make a hook to do all the formik field stuff

6c324dc

default to requiring submit before showing errors

148f25d

use new useHarmonyField on change password

2e1d720

reset the form after each successful submit

04d3195

debouncing clarity

b77d41e

naming things

d7b666b

oops

bf19dc5

don't disable

47c447f

rickyrombo requested a review from dylanjeffers February 8, 2024 01:56

pull-request-size bot added the size/L label Feb 8, 2024

dylanjeffers reviewed Feb 8, 2024

View reviewed changes

packages/web/src/components/change-password/ChangePasswordModal.tsx Show resolved Hide resolved

rickyrombo added 5 commits February 9, 2024 14:28

Make the hook input agnostic

1fac27b

Merge remote-tracking branch 'origin/main' into mjp-useHarmonyField

5d54b1b

Merge remote-tracking branch 'origin/main' into mjp-useHarmonyField

55b0bba

use imperative methods

c392dea

Fix Selectable Pill

5d29b56

Fix ReactNative, transformOnBlur

b50ca70

rickyrombo marked this pull request as ready for review February 15, 2024 01:23

Merge remote-tracking branch 'origin/main' into mjp-useHarmonyField

6617022

rickyrombo requested a review from DejayJD February 16, 2024 18:32

DejayJD reviewed Feb 16, 2024

View reviewed changes

DejayJD approved these changes Feb 16, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into mjp-useHarmonyField

02c4ef1

dylanjeffers approved these changes Mar 1, 2024

View reviewed changes

setTouched promise

eea3282

rickyrombo merged commit c3f0a99 into main Mar 1, 2024
11 of 15 checks passed

rickyrombo deleted the mjp-useHarmonyField branch March 1, 2024 01:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

useHarmonyField #7514

useHarmonyField #7514

rickyrombo commented Feb 8, 2024 •

edited

Loading

audius-infra commented Feb 8, 2024

gitguardian bot commented Feb 14, 2024 •

edited

Loading

audius-infra commented Feb 14, 2024

audius-infra commented Feb 15, 2024

audius-infra commented Feb 16, 2024

DejayJD Feb 16, 2024

rickyrombo Feb 16, 2024

DejayJD Feb 16, 2024 •

edited

Loading

rickyrombo Feb 16, 2024

DejayJD Feb 16, 2024

rickyrombo Feb 16, 2024

DejayJD Feb 16, 2024

rickyrombo Feb 16, 2024

rickyrombo Feb 16, 2024

DejayJD Feb 16, 2024

DejayJD Feb 16, 2024

DejayJD Feb 16, 2024 •

edited

Loading

rickyrombo Feb 16, 2024

DejayJD left a comment

audius-infra commented Mar 1, 2024

dylanjeffers left a comment •

edited

Loading

dylanjeffers Mar 1, 2024

dylanjeffers Mar 1, 2024

dylanjeffers Mar 1, 2024

rickyrombo Mar 1, 2024

dylanjeffers Mar 1, 2024

dylanjeffers Mar 1, 2024

		const maybePromise = setValue(value, false) as Promise<any> \| void
		if (maybePromise) {

useHarmonyField #7514

useHarmonyField #7514

Conversation

rickyrombo commented Feb 8, 2024 • edited Loading

Description

How Has This Been Tested?

audius-infra commented Feb 8, 2024

gitguardian bot commented Feb 14, 2024 • edited Loading

⚠️ GitGuardian has uncovered 4 secrets following the scan of your pull request.

audius-infra commented Feb 14, 2024

audius-infra commented Feb 15, 2024

audius-infra commented Feb 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DejayJD Feb 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DejayJD Feb 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DejayJD left a comment

Choose a reason for hiding this comment

audius-infra commented Mar 1, 2024

dylanjeffers left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickyrombo commented Feb 8, 2024 •

edited

Loading

gitguardian bot commented Feb 14, 2024 •

edited

Loading

DejayJD Feb 16, 2024 •

edited

Loading

DejayJD Feb 16, 2024 •

edited

Loading

dylanjeffers left a comment •

edited

Loading