diff --git a/packages/manager/.changeset/pr-13281-changed-1768479377489.md b/packages/manager/.changeset/pr-13281-changed-1768479377489.md new file mode 100644 index 00000000000..a7ee51e76d0 --- /dev/null +++ b/packages/manager/.changeset/pr-13281-changed-1768479377489.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Changed +--- + +Apply UX and user feedback for linode interfaces feature in Create linode page ([#13281](https://github.com/linode/manager/pull/13281)) diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Addons/Addons.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Addons/Addons.tsx index 114468f1754..6021bd0ca38 100644 --- a/packages/manager/src/features/Linodes/LinodeCreate/Addons/Addons.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreate/Addons/Addons.tsx @@ -1,7 +1,7 @@ import { useRegionsQuery } from '@linode/queries'; import { Divider, Notice, Paper, Stack, Typography } from '@linode/ui'; import React, { useMemo } from 'react'; -import { useWatch } from 'react-hook-form'; +import { useFormContext, useWatch } from 'react-hook-form'; import { Backups } from './Backups'; import { PrivateIP } from './PrivateIP'; @@ -9,6 +9,7 @@ import { PrivateIP } from './PrivateIP'; import type { CreateLinodeRequest } from '@linode/api-v4'; export const Addons = () => { + const { setValue } = useFormContext(); const [regionId, interfaceGeneration] = useWatch< CreateLinodeRequest, ['region', 'interface_generation'] @@ -26,6 +27,11 @@ export const Addons = () => { const shouldShowPrivateIP = interfaceGeneration !== 'linode'; + // Clean up private IP value when the option is hidden + if (!shouldShowPrivateIP) { + setValue('private_ip', false); + } + return ( diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Addons/Backups.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Addons/Backups.tsx index 758f683f2b1..e22a8e8b213 100644 --- a/packages/manager/src/features/Linodes/LinodeCreate/Addons/Backups.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreate/Addons/Backups.tsx @@ -108,7 +108,7 @@ export const Backups = () => { Three backup slots are executed and rotated automatically: a daily backup, a 2-7 day old backup, and an 8-14 day old backup. - Plans are priced according to the Linode plan selected above. + Pricing is based on the selected Linode plan. )} diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Addons/PrivateIP.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Addons/PrivateIP.tsx index a1a8ca2876a..7c5fdd791b1 100644 --- a/packages/manager/src/features/Linodes/LinodeCreate/Addons/PrivateIP.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreate/Addons/PrivateIP.tsx @@ -2,7 +2,6 @@ import { useRegionsQuery } from '@linode/queries'; import { Checkbox, FormControlLabel, - NewFeatureChip, Notice, Stack, Typography, @@ -59,14 +58,12 @@ export const PrivateIP = () => { Lets you connect with other Linodes in the same region over the data center's private network, without using a public IPv4 address. - - - - - You can use VPC for private networking instead. NodeBalancers - now connect to backend nodes without a private IPv4 address. - - + + + You can now establish network isolation and connections to + NodeBalancer backends through VPC. We recommend using VPC instead + of Private IPs. + } diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceGeneration.test.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceGeneration.test.tsx index ae7dbecf155..0ea189ce564 100644 --- a/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceGeneration.test.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceGeneration.test.tsx @@ -1,3 +1,4 @@ +import { waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; @@ -7,6 +8,8 @@ import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers'; import { InterfaceGeneration } from './InterfaceGeneration'; +const getAccountSettingsAPI = '*/v4*/account/settings'; + describe('InterfaceGeneration', () => { it('disables the radios if the account setting enforces linode_only interfaces', async () => { const accountSettings = accountSettingsFactory.build({ @@ -14,29 +17,25 @@ describe('InterfaceGeneration', () => { }); server.use( - http.get('*/v4*/account/settings', () => { + http.get(getAccountSettingsAPI, () => { return HttpResponse.json(accountSettings); }) ); - const { findByText, getAllByRole, getByText } = - renderWithThemeAndHookFormContext({ - component: , - }); - - const button = getByText('Network Interface Type'); - - // Expand the "Show More" - await userEvent.click(button); + const { getAllByRole, findByRole } = renderWithThemeAndHookFormContext({ + component: , + }); - // Hover to check for the tooltip - await userEvent.hover(getByText('Network Interface Type')); + // Wait for the tooltip icon to appear (indicating the disabled state) + await findByRole('button', { + name: 'Your account administrator has enforced that all new Linodes are created with Linode interfaces.', + }); - await findByText( - 'Your account administrator has enforced that all new Linodes are created with Linode interfaces.' - ); + // Verify both radio buttons are disabled + const radios = getAllByRole('radio'); + expect(radios).toHaveLength(2); - for (const radio of getAllByRole('radio')) { + for (const radio of radios) { expect(radio).toBeDisabled(); } }); @@ -47,34 +46,140 @@ describe('InterfaceGeneration', () => { }); server.use( - http.get('*/v4*/account/settings', () => { + http.get(getAccountSettingsAPI, () => { return HttpResponse.json(accountSettings); }) ); - const { findByText, getAllByRole, getByText } = - renderWithThemeAndHookFormContext({ - component: , - }); + const { getAllByRole, findByRole } = renderWithThemeAndHookFormContext({ + component: , + }); + + // Wait for the tooltip icon to appear (indicating the disabled state) + await findByRole('button', { + name: 'Your account administrator has enforced that all new Linodes are created with legacy configuration interfaces.', + }); - const button = getByText('Network Interface Type'); + // Verify both radio buttons are disabled + const radios = getAllByRole('radio'); + expect(radios).toHaveLength(2); - // Expand the "Show More" - await userEvent.click(button); + for (const radio of radios) { + expect(radio).toBeDisabled(); + } + }); - // Hover to check for the tooltip - await userEvent.hover(getByText('Network Interface Type')); + it('enables the radios when account settings allow both interface types', async () => { + const accountSettings = accountSettingsFactory.build({ + interfaces_for_new_linodes: 'linode_default_but_legacy_config_allowed', + }); - await findByText( - 'Your account administrator has enforced that all new Linodes are created with legacy configuration interfaces.' + server.use( + http.get(getAccountSettingsAPI, () => { + return HttpResponse.json(accountSettings); + }) ); - const radios = getAllByRole('radio'); + const { getAllByRole, queryByRole } = renderWithThemeAndHookFormContext({ + component: , + }); + // Wait for radios to render + await waitFor(() => { + // Verify no disabled tooltip appears + expect( + queryByRole('button', { + name: /Your account administrator has enforced/, + }) + ).toBeNull(); + }); + + // Verify both radio buttons are enabled + const radios = getAllByRole('radio'); expect(radios).toHaveLength(2); for (const radio of radios) { - expect(radio).toBeDisabled(); + expect(radio).toBeEnabled(); } }); + + it('defaults to linode interface when value is not set', async () => { + const accountSettings = accountSettingsFactory.build({ + interfaces_for_new_linodes: 'linode_default_but_legacy_config_allowed', + }); + + server.use( + http.get(getAccountSettingsAPI, () => { + return HttpResponse.json(accountSettings); + }) + ); + + const { getByDisplayValue } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + interface_generation: null, + }, + }, + }); + + // Wait for component to render + await waitFor(() => { + // Verify linode radio is selected by default + expect(getByDisplayValue('linode')).toBeChecked(); + }); + }); + + it('allows user to select legacy config interface when enabled', async () => { + const accountSettings = accountSettingsFactory.build({ + interfaces_for_new_linodes: 'legacy_config_default_but_linode_allowed', + }); + + server.use( + http.get(getAccountSettingsAPI, () => { + return HttpResponse.json(accountSettings); + }) + ); + + const { getByDisplayValue } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + interface_generation: 'linode', + }, + }, + }); + + const legacyConfigRadio = getByDisplayValue('legacy_config'); + + // Click on legacy config radio + await userEvent.click(legacyConfigRadio); + + // Verify legacy config is now selected + expect(legacyConfigRadio).toBeChecked(); + expect(getByDisplayValue('linode')).not.toBeChecked(); + }); + + it('displays correct labels for both interface types', () => { + const accountSettings = accountSettingsFactory.build({ + interfaces_for_new_linodes: 'linode_default_but_legacy_config_allowed', + }); + + server.use( + http.get(getAccountSettingsAPI, () => { + return HttpResponse.json(accountSettings); + }) + ); + + const { getByText } = renderWithThemeAndHookFormContext({ + component: , + }); + + // Verify interface type labels + expect(getByText('Linode Interfaces (Recommended)')).toBeVisible(); + expect( + getByText('Configuration Profile Interfaces (Legacy)') + ).toBeVisible(); + expect(getByText('Network Interface Type')).toBeVisible(); + }); }); diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceGeneration.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceGeneration.tsx index 9020bfd894b..90360222e93 100644 --- a/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceGeneration.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceGeneration.tsx @@ -9,13 +9,10 @@ import { TooltipIcon, Typography, } from '@linode/ui'; +import { FormLabel } from '@mui/material'; import React from 'react'; import { useController } from 'react-hook-form'; -import { ShowMoreExpansion } from 'src/components/ShowMoreExpansion'; - -import { LinodeInterfaceFeatureStatusChip } from '../../LinodesDetail/LinodeNetworking/LinodeInterfaces/LinodeInterfaceFeatureChip'; - import type { LinodeCreateFormValues } from '../utilities'; import type { LinodeInterfaceAccountSetting } from '@linode/api-v4'; @@ -45,89 +42,106 @@ export const InterfaceGeneration = () => { const disabled = disabledReason !== undefined; return ( - - + + + Network Interface Type + + {disabled && ( + + )} + + - } + data-qa-interfaces-option="linode" disabled={disabled} - sx={{ mt: '0px !important', mb: 2, mx: 0.5 }} - > - - } - data-qa-interfaces-option="linode" - label={ - - - ({ font: theme.font.bold })}> - Linode Interfaces + label={ + + + ({ font: theme.font.bold })}> + Linode Interfaces (Recommended) + + + + For VPC or public networking setups and when private IPs + are not required. + + + Linode Interfaces are directly associated with the + compute instance for easier visibility and management. + + + Different Cloud Firewalls can be assigned to each VPC + and public interface. + + + } + tooltipPosition="right" + width={280} + /> + + + } + sx={{ alignItems: 'flex-start' }} + value="linode" + /> + } + data-qa-interfaces-option="legacy_config" + disabled={disabled} + label={ + + ({ font: theme.font.bold })}> + Configuration Profile Interfaces (Legacy) + + + For Linodes requiring a private IP. + + Configuration Profile Interfaces are a part of the + configuration profile. + + + The same Cloud Firewall is assigned to all non-VLAN + interfaces on the Linode. - - - Managed directly through a Linode's Network - settings. This is the recommended option. -
-
- Cloud Firewalls are assigned to individual VPC and - public interfaces. - - } - /> - - } - sx={{ alignItems: 'flex-start' }} - value="linode" - /> - } - data-qa-interfaces-option="legacy_config" - label={ - - ({ font: theme.font.bold })}> - Configuration Profile Interfaces (Legacy) - - - Interfaces are part of the Linode's Configuration - Profile. -
-
- Cloud Firewalls are applied at the Linode level and - automatically cover all non-VLAN interfaces in the - Configuration Profile. - - } - /> - - } - sx={{ alignItems: 'flex-start' }} - value="legacy_config" - /> - - - - + } + tooltipPosition="right" + width={280} + /> + + } + sx={{ alignItems: 'flex-start' }} + value="legacy_config" + /> + + ); }; diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceType.test.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceType.test.tsx new file mode 100644 index 00000000000..4e4147a7455 --- /dev/null +++ b/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceType.test.tsx @@ -0,0 +1,95 @@ +import userEvent from '@testing-library/user-event'; +import React from 'react'; + +import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers'; + +import { InterfaceType } from './InterfaceType'; + +import type { LinodeCreateFormValues } from '../utilities'; + +const defaultFormValues: Partial = { + interface_generation: 'linode', + linodeInterfaces: [ + { + purpose: 'public', + firewall_id: null, + vpc: null, + public: null, + vlan: null, + default_route: null, + }, + ], +}; + +describe('InterfaceType', () => { + it('renders all interface type options', () => { + const { getByText, getByRole } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: defaultFormValues, + }, + }); + + // Check that the form label is rendered + expect(getByText('Network Connection')).toBeVisible(); + + // Check that all three interface options are rendered + expect(getByText('Public Internet')).toBeVisible(); + expect(getByText('VPC')).toBeVisible(); + expect(getByText('VLAN')).toBeVisible(); + + // Check that radiogroup is rendered + expect(getByRole('radiogroup')).toBeVisible(); + }); + + it('renders tooltip icons for each interface type', () => { + const { getAllByRole } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: defaultFormValues, + }, + }); + + // Should have tooltip buttons for each interface option + const tooltipButtons = getAllByRole('button'); + expect(tooltipButtons.length).toBe(3); + }); + + it('selects the correct radio based on form value', () => { + const { getByDisplayValue } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + ...defaultFormValues, + linodeInterfaces: [ + { + purpose: 'vpc', + firewall_id: null, + }, + ], + }, + }, + }); + + expect(getByDisplayValue('vpc')).toBeChecked(); + }); + + it('allows user to change interface type selection', async () => { + const { getByDisplayValue } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: defaultFormValues, + }, + }); + + // Initially public should be selected + expect(getByDisplayValue('public')).toBeChecked(); + + // Click on VPC radio + await userEvent.click(getByDisplayValue('vpc')); + + // VPC should now be selected + expect(getByDisplayValue('vpc')).toBeChecked(); + expect(getByDisplayValue('public')).not.toBeChecked(); + }); +}); diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceType.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceType.tsx index fbf5da03b00..c4b449c4dd9 100644 --- a/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceType.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceType.tsx @@ -30,7 +30,7 @@ const interfaceTypes = [ label: 'Public Internet', purpose: 'public', description: - 'Connects your Linode to the internet, enabling traffic over the public IP address.', + 'Connects your Linode to the internet and allows traffic through its public IP address.', }, { label: 'VPC', @@ -42,7 +42,7 @@ const interfaceTypes = [ label: 'VLAN', purpose: 'vlan', description: - 'Connects your Linode to a private Layer 2 network for local communication with other Linodes.', + 'Connects your Linode to a private Layer 2 network, enabling local communication with other Linodes.', }, ] as const; @@ -108,7 +108,7 @@ export const InterfaceType = ({ index }: Props) => { return ( - + Network Connection {disabled && ( { status="info" sxTooltipIcon={{ p: 0, ml: 0.5 }} text={interfaceType.description} + tooltipPosition="right" /> } diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Networking/LinodeInterface.test.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Networking/LinodeInterface.test.tsx index 74602447795..0abd34dc8d2 100644 --- a/packages/manager/src/features/Linodes/LinodeCreate/Networking/LinodeInterface.test.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreate/Networking/LinodeInterface.test.tsx @@ -22,9 +22,18 @@ describe('LinodeInterface (Linode Interfaces)', () => { component: , }); - expect(getByText('Public Internet')).toBeInTheDocument(); - expect(getByText('VPC')).toBeInTheDocument(); - expect(getByText('VLAN')).toBeInTheDocument(); + expect(getByText('Public Internet')).toBeVisible(); + expect(getByText('VPC')).toBeVisible(); + expect(getByText('VLAN')).toBeVisible(); + }); + + it('renders radios for the interfaces (Linode interface, Config profile)', () => { + const { getByText } = renderWithThemeAndHookFormContext({ + component: , + }); + + expect(getByText(/Linode Interfaces/)).toBeVisible(); + expect(getByText(/Configuration Profile Interfaces/)).toBeVisible(); }); it('renders a Firewall select if "VPC" is selected', async () => { diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Networking/LinodeInterface.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Networking/LinodeInterface.tsx index c4698a38e8b..f6b7276b18d 100644 --- a/packages/manager/src/features/Linodes/LinodeCreate/Networking/LinodeInterface.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreate/Networking/LinodeInterface.tsx @@ -3,6 +3,7 @@ import React from 'react'; import { useFormContext, useWatch } from 'react-hook-form'; import { InterfaceFirewall } from './InterfaceFirewall'; +import { InterfaceGeneration } from './InterfaceGeneration'; import { InterfaceType } from './InterfaceType'; import { VLAN } from './VLAN'; import { VPC } from './VPC'; @@ -44,6 +45,7 @@ export const LinodeInterface = ({ index }: Props) => { /> )} + {interfaceType === 'vlan' && } {interfaceType === 'vpc' && } {interfaceGeneration === 'linode' && interfaceType !== 'vlan' && ( diff --git a/packages/manager/src/features/Linodes/LinodeCreate/Networking/Networking.tsx b/packages/manager/src/features/Linodes/LinodeCreate/Networking/Networking.tsx index 142f26397f8..b2820e15a38 100644 --- a/packages/manager/src/features/Linodes/LinodeCreate/Networking/Networking.tsx +++ b/packages/manager/src/features/Linodes/LinodeCreate/Networking/Networking.tsx @@ -3,7 +3,6 @@ import React from 'react'; import { useFieldArray, useFormContext, useWatch } from 'react-hook-form'; import { Firewall } from './Firewall'; -import { InterfaceGeneration } from './InterfaceGeneration'; import { LinodeInterface } from './LinodeInterface'; import type { LinodeCreateFormValues } from '../utilities'; @@ -31,7 +30,6 @@ export const Networking = () => { Networking - {errors.linodeInterfaces?.message && ( )}