Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change: [M3-8377, M3-8579] - Move Region section above Images in Linode Create and update default OS to Ubuntu 24.04 LTS #10858

Merged
merged 9 commits into from
Sep 20, 2024
Merged

change: [M3-8377, M3-8579] - Move Region section above Images in Linode Create and update default OS to Ubuntu 24.04 LTS #10858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
719be41
move region above images in linode create v2
hana-akamai Aug 29, 2024
49158a3
Added changeset: Move Region section above Images in Linode Create
hana-akamai Sep 9, 2024
186062b
memoize region component
hana-akamai Sep 10, 2024
2b98156
change default OS to Ubuntu 24.04 LTS
hana-akamai Sep 12, 2024
bde9f93
update changelog
hana-akamai Sep 12, 2024
8b17604
Merge branch 'develop' into M3-8377-move-region-to-top-create-flow
hana-akamai Sep 12, 2024
8a1f76a
Merge branch 'develop' into M3-8377-move-region-to-top-create-flow
hana-akamai Sep 17, 2024
8aa2412
fix create-linode-mobile e2e test
hana-akamai Sep 17, 2024
e2e09fe
address feedback
hana-akamai Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10858-changed-1725914163062.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Changed
---

Move Region section above Images in Linode Create and update default OS to Ubuntu 24.04 LTS ([#10858](https://github.com/linode/manager/pull/10858))
6 changes: 3 additions & 3 deletions packages/manager/cypress/e2e/core/linodes/create-linode-mobile.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Linode create mobile smoke', () => {
cy.viewport(viewport.width, viewport.height);
cy.visitWithLogin('/linodes/create');

linodeCreatePage.selectImage('Debian 11');
linodeCreatePage.selectImage('Ubuntu 24.04 LTS');
linodeCreatePage.selectRegionById(mockLinodeRegion.id);
linodeCreatePage.selectPlanCard('Shared CPU', 'Nanode 1 GB');
linodeCreatePage.setLabel(mockLinode.label);
Expand All @@ -39,7 +39,7 @@ describe('Linode create mobile smoke', () => {
.scrollIntoView()
.within(() => {
cy.findByText('Nanode 1 GB').should('be.visible');
cy.findByText('Debian 11').should('be.visible');
cy.findByText('Ubuntu 24.04 LTS').should('be.visible');
cy.findByText(mockLinodeRegion.label).should('be.visible');
});

Expand All @@ -52,7 +52,7 @@ describe('Linode create mobile smoke', () => {
cy.wait('@createLinode').then((xhr) => {
const requestBody = xhr.request.body;

expect(requestBody['image']).to.equal('linode/debian11');
expect(requestBody['image']).to.equal('linode/ubuntu24.04');
expect(requestBody['label']).to.equal(mockLinode.label);
expect(requestBody['region']).to.equal(mockLinodeRegion.id);
expect(requestBody['type']).to.equal('g6-nanode-1');
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/cypress/e2e/core/linodes/create-linode-view-code-snippet.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ describe('Create Linode', () => {

cy.findByLabelText('Linode Label').should(
'have.value',
`debian-${linodeRegion.id}`
`ubuntu-${linodeRegion.id}`
);

cy.findByLabelText('Linode Label')
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/cypress/e2e/core/linodes/linode-storage.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('linode storage tab', () => {
});

it('try to delete in use disk', () => {
const diskName = 'Debian 11 Disk';
const diskName = 'Ubuntu 24.04 LTS Disk';
cy.defer(() => createTestLinode({ booted: true })).then((linode) => {
interceptDeleteDisks(linode.id).as('deleteDisk');
cy.visitWithLogin(`linodes/${linode.id}/storage`);
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/cypress/e2e/core/linodes/resize-linode.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe('resize linode', () => {
{ securityMethod: 'vlan_no_internet' }
)
).then((linode) => {
const diskName = 'Debian 11 Disk';
const diskName = 'Ubuntu 24.04 LTS Disk';
const size = '50000'; // 50 GB

// Error flow when attempting to resize a linode to a smaller size without
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/cypress/e2e/core/oneClickApps/one-click-apps.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ describe('OneClick Apps (OCA)', () => {
description: 'Minecraft OCA',
ordinal: 10,
logo_url: 'assets/Minecraft.svg',
images: ['linode/debian11', 'linode/ubuntu22.04'],
images: ['linode/debian11', 'linode/ubuntu24.04'],
deployments_total: 18854,
deployments_active: 412,
is_public: true,
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/cypress/e2e/core/stackscripts/smoke-community-stackscripts.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const mockStackScripts: StackScript[] = [
'linode/rocky8',
'linode/debian11',
'linode/centos-stream9',
'linode/ubuntu22.04',
'linode/ubuntu24.04',
'linode/almalinux9',
'linode/rocky9',
],
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/cypress/support/util/linodes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export const createTestLinode = async (
const resolvedCreatePayload = {
...createLinodeRequestFactory.build({
booted: false,
image: 'linode/debian11',
image: 'linode/ubuntu24.04',
label: randomLabel(),
region: chooseRegion().id,
}),
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/features/Linodes/LinodeCreatev2/Region.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
import type { LinodeCreateFormValues } from './utilities';
import type { Region as RegionType } from '@linode/api-v4';

export const Region = () => {
export const Region = React.memo(() => {
const {
isDiskEncryptionFeatureEnabled,
} = useIsDiskEncryptionFeatureEnabled();
Expand Down Expand Up @@ -267,4 +267,4 @@ export const Region = () => {
)}
</Paper>
);
};
});
18 changes: 11 additions & 7 deletions packages/manager/src/features/Linodes/LinodeCreatev2/Tabs/Clone/Clone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,22 @@ import React from 'react';
import { Paper } from 'src/components/Paper';
import { Stack } from 'src/components/Stack';
import { Typography } from 'src/components/Typography';
import { Region } from 'src/features/Linodes/LinodeCreatev2/Region';

import { LinodeSelectTable } from '../../shared/LinodeSelectTable';
import { CloneWarning } from './CloneWarning';

export const Clone = () => {
return (
<Paper>
<Stack spacing={1}>
<Typography variant="h2">Select Linode to Clone From</Typography>
<CloneWarning />
<LinodeSelectTable enablePowerOff />
</Stack>
</Paper>
<Stack spacing={3}>
<Paper>
<Stack spacing={1}>
<Typography variant="h2">Select Linode to Clone From</Typography>
<CloneWarning />
<LinodeSelectTable enablePowerOff />
</Stack>
</Paper>
<Region />
</Stack>
);
};
48 changes: 26 additions & 22 deletions packages/manager/src/features/Linodes/LinodeCreatev2/Tabs/Images.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { Paper } from 'src/components/Paper';
import { Placeholder } from 'src/components/Placeholder/Placeholder';
import { Stack } from 'src/components/Stack';
import { Typography } from 'src/components/Typography';
import { Region } from 'src/features/Linodes/LinodeCreatev2/Region';
import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck';
import { useAllImagesQuery } from 'src/queries/images';
import { useRegionsQuery } from 'src/queries/regions/regions';
Expand Down Expand Up @@ -95,27 +96,30 @@ export const Images = () => {
}

return (
<Paper>
<Typography variant="h2">Choose an Image</Typography>
<Box alignItems="flex-end" display="flex" flexWrap="wrap" gap={2}>
<ImageSelectv2
disabled={isCreateLinodeRestricted}
errorText={fieldState.error?.message}
onBlur={field.onBlur}
onChange={onChange}
sx={{ width: '416px' }}
value={field.value}
variant="private"
/>
{showDistributedCapabilityNotice && (
<Stack alignItems="center" direction="row" pb={0.8} spacing={1}>
<DistributedRegionIcon height="21px" width="24px" />
<Typography>
Indicates compatibility with distributed compute regions.
</Typography>
</Stack>
)}
</Box>
</Paper>
<Stack spacing={3}>
<Region />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that Region is rendered in these components, it is more susceptible to re-renders if any state in this component changes.

Could we somehow place Region outside of these subcomponents. I think it would help ensure we keep Linode Create v2 performant. Also, rendering a single instance of Region at a higher level will keep Linode Create v2 easy to understand from a code perspective.

This might be tricky with how the Tabs work but maybe this is still possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bnussman-akamai Hmm what about memoizing the Region component?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would help, but I still think it would be great if we could keep one instance of <Region /> in the JSX so the JSX conceptually matches up with the UI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bnussman-akamai Pushed the memo change. I tried keeping one instance of Region / having it at a higher level but since it's not always in the same place, that makes it difficult. This was the cleanest solution I could come up

Copy link
Member

@bnussman-akamai bnussman-akamai Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhhh. I didn't realize this only applied to the Image tabs. My fault. I'll think if there are any other options

<Paper>
<Typography variant="h2">Choose an Image</Typography>
<Box alignItems="flex-end" display="flex" flexWrap="wrap" gap={2}>
<ImageSelectv2
disabled={isCreateLinodeRestricted}
errorText={fieldState.error?.message}
onBlur={field.onBlur}
onChange={onChange}
sx={{ width: '416px' }}
value={field.value}
variant="private"
/>
{showDistributedCapabilityNotice && (
<Stack alignItems="center" direction="row" pb={0.8} spacing={1}>
<DistributedRegionIcon height="21px" width="24px" />
<Typography>
Indicates compatibility with distributed compute regions.
</Typography>
</Stack>
)}
</Box>
</Paper>
</Stack>
);
};
2 changes: 2 additions & 0 deletions packages/manager/src/features/Linodes/LinodeCreatev2/Tabs/Marketplace/Marketplace.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { useState } from 'react';

import { Stack } from 'src/components/Stack';
import { Region } from 'src/features/Linodes/LinodeCreatev2/Region';

import { StackScriptImages } from '../StackScripts/StackScriptImages';
import { UserDefinedFields } from '../StackScripts/UserDefinedFields/UserDefinedFields';
Expand All @@ -18,6 +19,7 @@ export const Marketplace = () => {
<Stack data-testid="one-click-apps-container" spacing={2}>
<AppSelect onOpenDetailsDrawer={onOpenDetailsDrawer} />
<UserDefinedFields onOpenDetailsDrawer={onOpenDetailsDrawer} />
<Region />
<StackScriptImages />
<AppDetailDrawerv2
onClose={() => setDrawerStackScriptId(undefined)}
Expand Down
31 changes: 18 additions & 13 deletions packages/manager/src/features/Linodes/LinodeCreatev2/Tabs/OperatingSystems.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import { useController, useFormContext } from 'react-hook-form';

import { ImageSelectv2 } from 'src/components/ImageSelectv2/ImageSelectv2';
import { Paper } from 'src/components/Paper';
import { Stack } from 'src/components/Stack';
import { Typography } from 'src/components/Typography';
import { Region } from 'src/features/Linodes/LinodeCreatev2/Region';
import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck';

import { getGeneratedLinodeLabel } from '../utilities';
Expand Down Expand Up @@ -45,18 +47,21 @@ export const OperatingSystems = () => {
};

return (
<Paper>
<Typography variant="h2">Choose an OS</Typography>
<ImageSelectv2
disabled={isCreateLinodeRestricted}
errorText={fieldState.error?.message}
label="Linux Distribution"
onBlur={field.onBlur}
onChange={onChange}
placeholder="Choose a Linux distribution"
value={field.value}
variant="public"
/>
</Paper>
<Stack spacing={3}>
<Region />
<Paper>
<Typography variant="h2">Choose an OS</Typography>
<ImageSelectv2
disabled={isCreateLinodeRestricted}
errorText={fieldState.error?.message}
label="Linux Distribution"
onBlur={field.onBlur}
onChange={onChange}
placeholder="Choose a Linux distribution"
value={field.value}
variant="public"
/>
</Paper>
</Stack>
);
};
2 changes: 2 additions & 0 deletions packages/manager/src/features/Linodes/LinodeCreatev2/Tabs/StackScripts/StackScripts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';

import { Stack } from 'src/components/Stack';
import { Region } from 'src/features/Linodes/LinodeCreatev2/Region';

import { StackScriptImages } from './StackScriptImages';
import { StackScriptSelection } from './StackScriptSelection';
Expand All @@ -11,6 +12,7 @@ export const StackScripts = () => {
<Stack spacing={3}>
<StackScriptSelection />
<UserDefinedFields />
<Region />
<StackScriptImages />
</Stack>
);
Expand Down
2 changes: 0 additions & 2 deletions packages/manager/src/features/Linodes/LinodeCreatev2/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import { EUAgreement } from './EUAgreement';
import { Firewall } from './Firewall';
import { FirewallAuthorization } from './FirewallAuthorization';
import { Plan } from './Plan';
import { Region } from './Region';
import { getLinodeCreateResolver } from './resolvers';
import { Security } from './Security';
import { SMTP } from './SMTP';
Expand Down Expand Up @@ -225,7 +224,6 @@ export const LinodeCreatev2 = () => {
</SafeTabPanel>
</TabPanels>
</Tabs>
{params.type !== 'Backups' && <Region />}
<Plan />
<Details />
{params.type !== 'Clone Linode' && <Security />}
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/features/Linodes/LinodeCreatev2/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import type { FieldErrors } from 'react-hook-form';
/**
* This is the ID of the Image of the default OS.
*/
const DEFAULT_OS = 'linode/debian11';
const DEFAULT_OS = 'linode/ubuntu24.04';

/**
* This interface is used to type the query params on the Linode Create flow.
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/features/Linodes/LinodesCreate/AddonsPanel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ const props: AddonsPanelProps = {
group: '',
hypervisor: 'kvm',
id: 45329311,
image: 'linode/debian11',
image: 'linode/ubuntu24.04',
ipv4: ['192.168.139.183', '139.144.17.202'],
ipv6: '2600:3c04::f03c:93ff:fe75:0612/128',
label: 'debian-ca-central',
Expand Down
2 changes: 1 addition & 1 deletion ...anager/src/features/Linodes/LinodesCreate/ApiAwarenessModal/IntegrationsTabPanel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type { IntegrationsTabPanelProps } from './IntegrationsTabPanel';

const defaultProps: IntegrationsTabPanelProps = {
payLoad: {
image: 'linode/debian11',
image: 'linode/ubuntu24.04',
label: 'debian-us-ord-001',
region: 'us-ord',
root_pass: 'testpassword',
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/features/Linodes/LinodesCreate/ApiAwarenessModal/SDKTabPanel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type { SDKTabPanelProps } from './SDKTabPanel';

const defaultProps: SDKTabPanelProps = {
payLoad: {
image: 'linode/debian11',
image: 'linode/ubuntu24.04',
label: 'debian-us-ord-001',
region: 'us-ord',
root_pass: 'testpassword',
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/features/Linodes/LinodesCreate/LinodeCreateContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ import type { MapState } from 'src/store/types';
import type { ExtendedType } from 'src/utilities/extendType';
import type { ExtendedIP } from 'src/utilities/ipUtils';

const DEFAULT_IMAGE = 'linode/debian11';
const DEFAULT_IMAGE = 'linode/ubuntu24.04';

interface State {
additionalIPv4RangesForVPC: ExtendedIP[];
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/store/image/image.helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ describe('isLinodeKubeImageId', () => {
expect(isLinodeKubeImageId('linode/alpine3.15')).toBe(false);
});
it('should be true if the image is a linode kube image', () => {
expect(isLinodeKubeImageId('linode/debian11-kube-v1.23.4')).toBe(true);
expect(isLinodeKubeImageId('linode/ubuntu24.04-kube-v1.23.4')).toBe(true);
});
});
2 changes: 1 addition & 1 deletion packages/manager/src/store/image/image.helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const filterImagesByType = (
* LKE (Linode Kubernetes Engine) image. In Cloud Manager, we hide
* these images from the user.
*
* Image IDs are in the form linode/debian11 or private/15943292
* Image IDs are in the form linode/ubuntu24.04 or private/15943292
*
* @param {string | null} id the image's id (unlike most entities, image ids are string)
* @returns {boolean} true if the image is an LKE image
Expand Down
Loading