upcoming: [M3-7926] - Linode Create Refactor - VLANs - Part 9 #10342

bnussman-akamai · 2024-04-02T17:40:17Z

Description 📝

Adds VLANs to the new Linode Create flow ✨

Changes 🔄

Use query key factory for VLANs
Adds new VLANSelect component
Add infinite query for VLANs to power the VLAN Select
Adds VLANs section to the new Linode Create flow

Preview 📷

How to test 🧪

Prerequisites

Turn on the Linode Create v2 feature flag using our local dev tools

Verification steps

Test the new VLAN section
Test the new VLANSelect
Check new logic, utils, and UI for issues, regressions, and quality
Verify you can create a Linode with a VLAN attached
Verify VLANs throughout the app still work as expected

As an Author I have considered 🤔

github-actions · 2024-04-02T21:11:37Z

Coverage Report: ✅
Base Coverage: 81.66%
Current Coverage: 81.7%

dwiley-akamai

I observed a bit of odd behavior in the Linode Create flow with a "ghost" VLAN appearing in the dropdown:

Screen.Recording.2024-04-03.at.9.53.53.AM.mov

Additionally, based on the API docs, VLANs can only be attached to Linodes within the same DC. Our functionality in prod currently filters the VLAN dropdown based on region, but we don't have that on this branch

packages/manager/src/queries/linodes/linodes.ts

abailly-akamai

Looks good and well tested. Left some comments.

Also, maybe i am doing something wrong, but ran into this error when submitting

{
    "reason": "metadata dict must contain key user_data",
    "field": "metadata"
}

packages/manager/src/components/VLANSelect.tsx

packages/manager/src/features/Linodes/LinodeCreatev2/utilities.ts

cpathipa

Overall, LGTM I agree with the feedback from @abailly-akamai and @dwiley-akamai . Approving, pending the addressing of their feedback.

cpathipa · 2024-04-03T17:33:05Z

packages/manager/src/components/VLANSelect.tsx

+      loading={isLoading}
+      noOptionsText="You have no VLANs in this region. Type to create one."
+      options={vlans}
+      placeholder="Create or select a VLAN"


UX observation: Currently, the user will only know the loading state upon clicking the field. Could a dynamic placeholder help?

I'm hoping that the changes from #10122 improves the loading state

abailly-akamai

Looks great on my end, nice work! Thanks for addressing those changes as well ✅

packages/manager/src/components/VLANSelect.tsx

hkhalil-akamai · 2024-04-03T21:10:07Z

I observed a bit of odd behavior in the Linode Create flow with a "ghost" VLAN appearing in the dropdown:

Was there a response to this comment by @dwiley-akamai? I am not able to replicate it myself -- I think it might be different VLANs in different regions with the same name.

packages/manager/src/components/VLANSelect.test.tsx

packages/manager/src/components/VLANSelect.tsx

hkhalil-akamai · 2024-04-03T21:20:20Z

packages/manager/src/features/Linodes/LinodeCreatev2/VLAN.tsx

+  const disabled =
+    !imageId ||
+    isCreatingFromBackup ||
+    isLinodeCreateRestricted ||
+    !regionSupportsVLANs;


For parity with the current flow, the VLANs field should reset when disabled goes from false to true.

In the (admittedly contrived) case that the user selects a VLAN and then e.g., clears the image field, the form will still be submittable with the VLAN attached. This causes an API error and also breaks the form (this might be an unrelated), see attached video:

Screen.Recording.2024-04-03.at.5.18.14.PM.mov

Yes, good catch. This will have to be addressed at some point. I was holding off on this for now because there will be significant work dedicated to "resetting" form fields under certain conditions like what you showed and even switching tabs.

I need to find a clean way to handle these "side-effect" state changes. There are many cases similar to this

One idea might be with a useEffect as so:

React.useEffect(() => { if (disabled) { // Reset input here } }, [disabled]);

bnussman-akamai · 2024-04-03T21:24:09Z

I wasn't able to reproduce the "ghost" issue, but the component has undergone some significant changes and it now filters by region. I think we should be good now @hkhalil-akamai @dwiley-akamai

hkhalil-akamai

Thanks for addressing feedback @bnussman-akamai!

dwiley-akamai

Functionality LGTM now, no longer observing the ghost VLAN issue

dwiley-akamai · 2024-04-04T16:04:15Z

packages/manager/src/components/VLANSelect.tsx

+
+  const vlans = data?.pages.flatMap((page) => page.data) ?? [];
+
+  const newVlanPlacehodler = {


Suggested change

const newVlanPlacehodler = {

const newVlanPlaceholder = {

dwiley-akamai · 2024-04-04T16:06:34Z

packages/manager/src/features/Linodes/LinodeCreatev2/utilities.ts

  return payload;
 };
+
+/**
+ * Performans transformation and ordering on the Linode Create "interfaces" form data.


Suggested change

* Performans transformation and ordering on the Linode Create "interfaces" form data.

* Performs a transformation and ordering on the Linode Create "interfaces" form data.

dwiley-akamai · 2024-04-04T16:06:50Z

packages/manager/src/features/Linodes/LinodeCreatev2/utilities.ts

+/**
+ * Performans transformation and ordering on the Linode Create "interfaces" form data.
+ *
+ * We need this so we can put interfaces in the correct order and omit unused iterfaces.


Suggested change

* We need this so we can put interfaces in the correct order and omit unused iterfaces.

* We need this so we can put interfaces in the correct order and omit unused interfaces.

bnussman-akamai · 2024-04-04T16:37:42Z

Oops. I forgot to fix those spelling issues @dwiley-akamai. My fault 🤦

I'll make sure to fix them in the next Linode Create reactor PR

bnussman added 7 commits March 28, 2024 20:09

initial work

30412a7

Merge branch 'develop' into M3-7926-poc-1

1770e83

clean up

4c49482

more progress

865e4cc

more progress

64ee751

fix up vlan queries

4d42f1d

make it look like existing

29d00e5

bnussman-akamai added the Linode Create Refactor label Apr 2, 2024

bnussman-akamai self-assigned this Apr 2, 2024

bnussman added 4 commits April 2, 2024 14:15

make things match existing behavior and test

c88d156

test new vlan select

c41ff69

Merge branch 'develop' into M3-7926-poc-1

877e567

add summary item for VLAN

ae2c81e

bnussman-akamai marked this pull request as ready for review April 2, 2024 20:48

bnussman-akamai requested a review from a team as a code owner April 2, 2024 20:48

bnussman-akamai requested review from dwiley-akamai and hkhalil-akamai and removed request for a team April 2, 2024 20:48

Added changeset: Linode Create Refactor - VLANs - Part 9

fcdbd24

bnussman-akamai requested review from jaalah-akamai and abailly-akamai April 2, 2024 20:50

dwiley-akamai reviewed Apr 3, 2024

View reviewed changes

packages/manager/src/queries/linodes/linodes.ts Outdated Show resolved Hide resolved

only show VLANs in selected region

2a7217e

abailly-akamai reviewed Apr 3, 2024

View reviewed changes

bnussman added 5 commits April 3, 2024 12:49

feedback @abailly-akamai

5cb3f12

feedback @abailly-akamai

449e990

fix VLANSelect

3ee8950

improve VLANSelect

1a4cda3

dial in the new VLANSelect

7cc7150

continue to dial in new VLAN Select

fd25ada

cpathipa approved these changes Apr 3, 2024

View reviewed changes

bnussman-akamai added the Work in Progress label Apr 3, 2024

bnussman added 2 commits April 3, 2024 14:34

test VLANSelect util

b3f879d

don't use factory for placeholder VLAN

3d2600f

bnussman-akamai removed the Work in Progress label Apr 3, 2024

bnussman-akamai requested review from dwiley-akamai and abailly-akamai April 3, 2024 18:46

bnussman added 3 commits April 3, 2024 14:56

optimize loading

464cb50

Merge branch 'develop' into M3-7926-poc-1

73264c8

improve loading state

7a4ee7b

abailly-akamai approved these changes Apr 3, 2024

View reviewed changes

packages/manager/src/components/VLANSelect.tsx Outdated Show resolved Hide resolved

packages/manager/src/components/VLANSelect.tsx Show resolved Hide resolved

packages/manager/src/components/VLANSelect.tsx Show resolved Hide resolved

hkhalil-akamai reviewed Apr 3, 2024

View reviewed changes

feedback

f2e8800

hkhalil-akamai approved these changes Apr 4, 2024

View reviewed changes

dwiley-akamai approved these changes Apr 4, 2024

View reviewed changes

bnussman-akamai merged commit 774c060 into linode:develop Apr 4, 2024
18 checks passed

dwiley-akamai mentioned this pull request Apr 10, 2024

upcoming: [M3-7924] - Linode Create Refactor - VPC - Part 10 #10354

Merged

12 tasks

This was referenced Apr 11, 2024

Release v1.117.0 - release → staging #10368

Merged

Release v1.117.0 - staging → master #10377

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upcoming: [M3-7926] - Linode Create Refactor - VLANs - Part 9 #10342

upcoming: [M3-7926] - Linode Create Refactor - VLANs - Part 9 #10342

bnussman-akamai commented Apr 2, 2024 •

edited

github-actions bot commented Apr 2, 2024 •

edited

dwiley-akamai left a comment

abailly-akamai left a comment

cpathipa left a comment •

edited

cpathipa Apr 3, 2024

bnussman-akamai Apr 3, 2024

abailly-akamai left a comment

hkhalil-akamai commented Apr 3, 2024

hkhalil-akamai Apr 3, 2024

bnussman-akamai Apr 3, 2024 •

edited

hkhalil-akamai Apr 4, 2024

bnussman-akamai commented Apr 3, 2024

hkhalil-akamai left a comment

dwiley-akamai left a comment

dwiley-akamai Apr 4, 2024

dwiley-akamai Apr 4, 2024

dwiley-akamai Apr 4, 2024

bnussman-akamai commented Apr 4, 2024


		const vlans = data?.pages.flatMap((page) => page.data) ?? [];

		const newVlanPlacehodler = {

	* Performans transformation and ordering on the Linode Create "interfaces" form data.
	* Performs a transformation and ordering on the Linode Create "interfaces" form data.

	* We need this so we can put interfaces in the correct order and omit unused iterfaces.
	* We need this so we can put interfaces in the correct order and omit unused interfaces.

upcoming: [M3-7926] - Linode Create Refactor - VLANs - Part 9 #10342

upcoming: [M3-7926] - Linode Create Refactor - VLANs - Part 9 #10342

Conversation

bnussman-akamai commented Apr 2, 2024 • edited

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Prerequisites

Verification steps

As an Author I have considered 🤔

github-actions bot commented Apr 2, 2024 • edited

dwiley-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

cpathipa left a comment • edited

Choose a reason for hiding this comment

cpathipa Apr 3, 2024

Choose a reason for hiding this comment

bnussman-akamai Apr 3, 2024

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

hkhalil-akamai commented Apr 3, 2024

hkhalil-akamai Apr 3, 2024

Choose a reason for hiding this comment

bnussman-akamai Apr 3, 2024 • edited

Choose a reason for hiding this comment

hkhalil-akamai Apr 4, 2024

Choose a reason for hiding this comment

bnussman-akamai commented Apr 3, 2024

hkhalil-akamai left a comment

Choose a reason for hiding this comment

dwiley-akamai left a comment

Choose a reason for hiding this comment

dwiley-akamai Apr 4, 2024

Choose a reason for hiding this comment

dwiley-akamai Apr 4, 2024

Choose a reason for hiding this comment

dwiley-akamai Apr 4, 2024

Choose a reason for hiding this comment

bnussman-akamai commented Apr 4, 2024

bnussman-akamai commented Apr 2, 2024 •

edited

github-actions bot commented Apr 2, 2024 •

edited

cpathipa left a comment •

edited

bnussman-akamai Apr 3, 2024 •

edited