refactor(components): Expose Reusable hooks and components used in FormField [JOB-110812] #2184

MichaelParadis · 2024-12-02T17:38:36Z

Copied from: #2105

Motivations

We have an upcoming need to decouple from react-hook-form. We still want to leverage the library, but we no longer want our form components to be directly coupled, instead we want them to support being used in that environment but not use the library directly.

This PR is the first step towards that future.

In this PR, we've broken down all the requisite parts of FormField and FormFieldWrapper into either smaller bite-sized components, or hooks.

This allows FormFieldWrapper to now just be collating a variety of smaller pieces into a cohesive whole. The purpose of the first part of this refactor was to ensure that nothing changed for the main control flow, which can be verified by our passing test suite (with no changes to tests in this PR. I want to verify passing tests still in our downstream applications).

With FormFieldWrapper/FormField broken down into chunks, we could now leverage those chunks directly in a new version of InputText that bypasses the Form and react-hook-form specific FormField components by leveraging FormFieldWrapper directly, and providing the input html element directly.

You can verify this new component is working through the next step in this refactor which is using the potential pattern for an Atlantis Shim through the use of a version prop on the input to control if the original implementation of the Input or a new version

Testing

Verify styling of the base inputs matches master (you can revert 6643db603a804552d6c46fbd4ed16e3f83509fbe for a comparison story on InputText

Changes

Refactored FormFieldWrapper and FormField to be broken down into smaller chunks, but no actual code changes.
Added a potential pattern for an Atlantis Shim to allow existing users of a component to get the legacy version of the component, and consumers wishing to use the new pattern can apply an additional prop version:2 to the prop list.

No visual Changes

After (this branch):

No value

With value (clearable)

With value (not clearable)

Before (Master)

No value

With value (clearable)

With value (not clearable)

In Atlantis we use Github's built in pull request reviews.

…he middle

cloudflare-workers-and-pages · 2024-12-02T17:41:36Z

Deploying atlantis with Cloudflare Pages

Latest commit:	`6d4ace1`
Status:	✅ Deploy successful!
Preview URL:	https://62de306c.atlantis.pages.dev
Branch Preview URL:	https://mike-pull-scotts-rfh-formfie.atlantis.pages.dev

View logs

MichaelParadis · 2024-12-02T17:55:46Z

packages/components/src/InputValidation/InputValidation.tsx

-export function InputValidation({ message }: InputValidationProps) {
+export function InputValidation({
+  message,
+  visible = true,


Not a breaking change. If people use InputValidation it will render as before but they can now toggle with visible to use it in a new way. See: https://github.com/GetJobber/atlantis/pull/2184/files#diff-2dd45bcb75b2965ffe0b5fceb22778aaea0eb9af7c9912de07dae06ff3f7bafeR122-R127

Should we update the storybook examples, add some tests? (I'm open to that being a followup PR! Let's just not forget, lol.)

I can push some tests and update the storybook example. The commit should be very small

MichaelParadis · 2024-12-02T17:56:06Z

packages/components/src/FormField/FormFieldDescription.tsx

 }

 export function FormFieldDescription({
  id,
  description,
+  visible = true,


See https://github.com/GetJobber/atlantis/pull/2184/files#r1866334441

MichaelParadis · 2024-12-02T17:58:19Z

packages/components/src/FormField/FormFieldAffix.tsx

@@ -19,6 +19,7 @@ export function AffixLabel({
  const affixLabelClass = classnames(styles.affixLabel, {
    [styles.suffix]: variation === "suffix",
  });
+  if (!label) return null;


This might actually break things. confirm this

There was a breaking change but I fixed it in d2db62b

After (this branch):

No value

With value (clearable)

With value (not clearable)

Before (Master)

No value

With value (clearable)

With value (not clearable)

Thanks for catching these!

This reverts commit d2db62b.

MichaelParadis · 2024-12-02T19:55:02Z

packages/components/src/FormField/FormField.test.tsx

@@ -229,7 +229,7 @@ describe("FormField", () => {
    describe("with validation errors", () => {
      it("should trigger onValidation with error message", async () => {
        const validationHandler = jest.fn();
-        const validate = val => (val == "Bob" ? "message" : "foo");
+        const validate = (val: string) => (val == "Bob" ? "message" : "foo");


Typescript was upset so I added this

github-actions · 2024-12-02T22:10:33Z

Published Pre-release for 7d365c4 with versions:

  - @jobber/[email protected]+7d365c4c

To install the new version(s) for Web run:

npm install @jobber/[email protected]+7d365c4c

scotttjob

I know this is still in Draft, but I wanted to give it an approval to give you the go ahead for this work.

Ultimately, the idea is we're changing absolutely nothing. Just re-arranging the code a bit, before we actually start changing things in the V2 versions of some of our form components.

So as long as you get the green light in all our downstream CI, I think we can move forward with this change. We might want to put out a bit of communication before we roll this out, but it doesn't necessitate a major version bump by any means.

scotttjob · 2024-12-03T11:45:24Z

packages/components/src/FormField/FormField.tsx

@@ -26,7 +20,6 @@ type FormFieldInternalProps = FormFieldProps & {
  readonly id: string;
 };

-// eslint-disable-next-line max-statements


This makes me happy right out of the gate.

scotttjob · 2024-12-03T11:45:37Z

packages/components/src/FormField/FormField.tsx

-      setValue(name, newValue, { shouldValidate: true });
-    },
-  }));
+  const { name } = useAtlantisFormFieldName({ id, nameProp });


scotttjob · 2024-12-03T11:46:44Z

packages/components/src/FormField/FormField.tsx

-    },
-    fieldState: { error },
-  } = useController({
+    errorMessage,


It looks long, but ultimately there's so much power being unlocked here! :)

scotttjob · 2024-12-03T11:47:11Z

packages/components/src/FormField/FormField.tsx

-    onKeyDown: handleKeyDown,
-  };
-  const inputRefs = mergeRefs([inputRef, fieldRef]);
+  const { textFieldProps, fieldProps, descriptionIdentifier } =


I know we're just shuffling code around ultimately, but it LOOKS nicer on top of being more extensible. :)

scotttjob · 2024-12-03T11:48:16Z

packages/components/src/FormField/FormFieldAffix.tsx

@@ -19,6 +19,7 @@ export function AffixLabel({
  const affixLabelClass = classnames(styles.affixLabel, {
    [styles.suffix]: variation === "suffix",
  });
+  if (!label) return null;


Thanks for catching these!

scotttjob · 2024-12-03T11:51:06Z

packages/components/src/FormField/FormFieldWrapper.tsx

+              placeholder={placeholder}
+              identifier={identifier}
+              style={
+                prefixRef?.current || suffixRef?.current


This is the only part of this I'm not huge on, we can probably pull that into one of these hooks, but it's not a big enough deal to hold up this particular train. Let's keep rolling!

scotttjob · 2024-12-03T11:51:43Z

packages/components/src/FormField/FormFieldWrapper.tsx

        />
-      )}
-      {error && !inline && <InputValidation message={error} />}
+      </div>


A V2 comment: Is there an opportunity to get rid of these Divs and leverage our own components instead?

scotttjob · 2024-12-03T11:52:38Z

packages/components/src/FormField/hooks/useAtlantisFormField.ts

+    autoFocus: autofocus,
+    onKeyDown: handleKeyDown,
+  };
+  useEffect(() => handleValidation(errorMessage), [errorMessage]);


Looks great!

MichaelParadis · 2024-12-03T15:52:25Z

@scotttjob I noticed that the clearing isn't focusing the input once it is clicked. I think I know why but I am going to spend some time fixing that

MichaelParadis · 2024-12-03T15:58:02Z

It looks like I figured out the issue

…gressions

packages/components/src/FormField/hooks/useAtlantisReactHookForm.ts

JeffMarvin

Great stuff here! I had a couple of comments so I thought I would provide those now.

I've checked the components in storybook and they all seem to be behaving. I want to run a bit of QA on the JO PR you have and on the shim behavior.

packages/components/src/FormField/FormFieldTypes.ts

JeffMarvin · 2024-12-03T21:09:12Z

packages/components/src/InputValidation/InputValidation.tsx

-export function InputValidation({ message }: InputValidationProps) {
+export function InputValidation({
+  message,
+  visible = true,


Should we update the storybook examples, add some tests? (I'm open to that being a followup PR! Let's just not forget, lol.)

Added test for testing the visible prop

scotttjob and others added 2 commits December 2, 2024 09:35

exploring what our forms could look like without react-hook-form in t…

298108a

…he middle

Pull FormField changes into reusable hooks

4fb9122

MichaelParadis force-pushed the MIKE/pull-scotts-rfh-formfield-changes branch from 174fb2e to dfa93ca Compare December 2, 2024 17:49

Undo adding of InputText.rebuilt

7c4b2fa

MichaelParadis force-pushed the MIKE/pull-scotts-rfh-formfield-changes branch from dfa93ca to 7c4b2fa Compare December 2, 2024 17:52

Undo unneeded change to meta.json

464165a

MichaelParadis commented Dec 2, 2024

View reviewed changes

MichaelParadis added 5 commits December 2, 2024 11:03

Fix eslint error

384cfad

Fix eslint error

b93d5cc

Fix prefix / suffix offset calculation issue

636e553

Commit for doing comparisons

d2db62b

Revert "Commit for doing comparisons"

6643db6

This reverts commit d2db62b.

MichaelParadis commented Dec 2, 2024

View reviewed changes

MichaelParadis changed the title ~~feat(components): RHF formfield changes~~ refactor(components): Expose Reusable hooks and components used in FormField Dec 2, 2024

MichaelParadis added 4 commits December 2, 2024 14:05

Cleaning up types in useAtlantisFormField

f154acc

Cleanup types in useAtlantisReactHookForm

83843fe

Cleanup props in useAtlantisFormFieldActions

42ac4da

Cleanup types in useFormFieldWrapperStyles

7b61b13

MichaelParadis requested a review from JeffMarvin December 2, 2024 22:07

jobbiebot bot added the review requested label Dec 2, 2024

MichaelParadis removed the request for review from JeffMarvin December 2, 2024 22:07

scotttjob approved these changes Dec 3, 2024

View reviewed changes

jobbiebot bot added approved and removed review requested labels Dec 3, 2024

MichaelParadis changed the title ~~refactor(components): Expose Reusable hooks and components used in FormField~~ refactor(components): Expose Reusable hooks and components used in FormField [JOB-110812] Dec 3, 2024

MichaelParadis marked this pull request as ready for review December 3, 2024 15:04

MichaelParadis requested a review from a team as a code owner December 3, 2024 15:04

jobbiebot bot added review requested Team Atlantis labels Dec 3, 2024

MichaelParadis requested a review from JeffMarvin December 3, 2024 15:05

Fix formfield not focusing when cleared. Added new test to prevent re…

7d365c4

…gressions

JeffMarvin reviewed Dec 3, 2024

View reviewed changes

packages/components/src/FormField/hooks/useAtlantisReactHookForm.ts Outdated Show resolved Hide resolved

Rename useAtlantisReactForm to useAtlantisReactHookForm

64c0733

JeffMarvin reviewed Dec 3, 2024

View reviewed changes

MichaelParadis added 2 commits December 3, 2024 13:30

Add example of using Visible prop of InputValidation

ffaab53

Added test for testing the visible prop

Update comment on version property in our FormFieldProps

6d4ace1

JeffMarvin approved these changes Dec 4, 2024

View reviewed changes

jobbiebot bot removed the review requested label Dec 4, 2024

MichaelParadis merged commit d0c4be1 into master Dec 4, 2024
13 checks passed

MichaelParadis deleted the MIKE/pull-scotts-rfh-formfield-changes branch December 4, 2024 20:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(components): Expose Reusable hooks and components used in FormField [JOB-110812] #2184

refactor(components): Expose Reusable hooks and components used in FormField [JOB-110812] #2184

MichaelParadis commented Dec 2, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Dec 2, 2024 •

edited

Loading

MichaelParadis Dec 2, 2024

JeffMarvin Dec 3, 2024

MichaelParadis Dec 3, 2024

MichaelParadis Dec 2, 2024

MichaelParadis Dec 2, 2024

MichaelParadis Dec 2, 2024 •

edited

Loading

scotttjob Dec 3, 2024

MichaelParadis Dec 2, 2024

github-actions bot commented Dec 2, 2024 •

edited

Loading

scotttjob left a comment

scotttjob Dec 3, 2024

scotttjob Dec 3, 2024

scotttjob Dec 3, 2024

scotttjob Dec 3, 2024

scotttjob Dec 3, 2024

scotttjob Dec 3, 2024

scotttjob Dec 3, 2024

scotttjob Dec 3, 2024

MichaelParadis commented Dec 3, 2024

MichaelParadis commented Dec 3, 2024

JeffMarvin left a comment

JeffMarvin Dec 3, 2024

refactor(components): Expose Reusable hooks and components used in FormField [JOB-110812] #2184

refactor(components): Expose Reusable hooks and components used in FormField [JOB-110812] #2184

Conversation

MichaelParadis commented Dec 2, 2024 • edited Loading

Motivations

Testing

Changes

No visual Changes

After (this branch):

Before (Master)

cloudflare-workers-and-pages bot commented Dec 2, 2024 • edited Loading

Deploying atlantis with Cloudflare Pages

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

MichaelParadis Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

After (this branch):

Before (Master)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 2, 2024 • edited Loading

scotttjob left a 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

Choose a reason for hiding this comment

MichaelParadis commented Dec 3, 2024

MichaelParadis commented Dec 3, 2024

JeffMarvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelParadis commented Dec 2, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Dec 2, 2024 •

edited

Loading

MichaelParadis Dec 2, 2024 •

edited

Loading

github-actions bot commented Dec 2, 2024 •

edited

Loading