upcoming: [M3-8638] - Introduce the new `(at)linode/ui` package #11057

hkhalil-akamai · 2024-10-07T21:32:28Z

Description 📝

Introduces the new @linode/ui package, which will host our shared UI component library. The package also supports unit tests via vitest and will eventually become the home of our Storybook declarations.

This is the first step in our modularization effort. These components will eventually be published and used outside of Cloud Manager, such as in downstream microfrontends. Therefore, they need to function independently of any tools, utilities or components specific to Cloud Manager.

The package must support unit testing through vitest. Existing unit tests will be migrated alongside their associated components.

Storybook and all storybook declarations will also eventually be hosted from this package.

Changes 🔄

Introduces the new @linode/ui package
Update generate-changelog and package-version scripts to include this package
Update ci.yml to typecheck and test this package
Copy the Chip and BetaChip components to demonstrate the usage of this package

How to test 🧪

Importing components from this package into Cloud Manager works as expected
Type checking passes
All unit tests pass
Release scripts continue to work as expected with this package (changelog, package versions)
CI passes -- the package passes typechecking and unit testing

bnussman-akamai

Just realized this was a draft 🤦 Going to leave this early feedback so it doesn't get lost but will wait to continue reviewing, sorry about that!

packages/ui/package.json

packages/ui/src/components/BetaChip/BetaChip.test.tsx

hkhalil-akamai · 2024-10-08T18:48:22Z

packages/ui/.eslintrc.json

Copied from packages/api-v4/.eslintrc.json

hkhalil-akamai · 2024-10-08T18:51:49Z

packages/ui/.prettierrc

Copied from packages/api-v4/.prettierrc

hkhalil-akamai · 2024-10-08T18:52:54Z

packages/ui/src/components/BetaChip/BetaChip.tsx

+    padding: 0,
+  },
+  fontSize: '0.625rem',
+  fontFamily: '"LatoWebBold", sans-serif', // TODO: remove hardcoded font once theme is added to this package


theme.font is a custom declaration from our theme.ts file

jaalah-akamai · 2024-10-08T20:35:53Z

packages/ui/src/components/Chip.tsx

+   * Optional component to render instead of a span.
+   */
+  component?: React.ElementType;
+}


Something we should address in a follow-up. I don't remember why we ever needed this abstraction, but the _ChipProps already has component as part of it's type.

Figured this was small enough to include in this PR 07e44

jaalah-akamai

This is looking great. Approving pending addition of CHANGELOG.md.

✅ Changesets are working in package
✅ yarn package-versions works as intended

✅ Configs / Dependencies look good
❌ Need to create a CHANGELOG.md file for yarn generate-changelogs

github-actions · 2024-10-08T21:11:20Z

Coverage Report: ✅
Base Coverage: 86.96%
Current Coverage: 86.96%

mjac0bs

Importing components from this package into Cloud Manager works as expected - confirmed chips look good
Type checking passes
All unit tests pass - left a ❓ about running tests locally
Release scripts continue to work as expected with this package (changelog, package versions) - no issues other than what Jaalah already noted 👍🏼
CI passes -- the package passes typechecking and unit testing

README.md

mjac0bs · 2024-10-08T21:35:41Z

packages/ui/vitest.config.ts

Minor and non-blocking: Is there anything we can change in our root package.json to be able to run tests from both manager and ui packages? (Edit: yeah, yarn workspace linode-manager test && yarn workspace @linode/ui test should do it.)

It looks like we have "test": "yarn workspace linode-manager test" configured in the root package.json scripts, but when I ran yarn test from the root, I was only seeing manager tests run.

Navigating into the ui package and running yarn test from there worked as expected.

I have some reservations about &&ing these two commands together since it might make it hard to read the output of the test command. For example, failing Manager tests would be hidden somewhere in the middle of the command output.

I wonder if there is a way to instruct vitest to run both test suites as if they are one -- curious if @bnussman-akamai has any suggestions here.

I also am anti-&&

I think we should just use commands like yarn test:manager and yarn test:ui

I don't really know of anything else more elegant we can do 🫤

For example, failing Manager tests would be hidden somewhere in the middle of the command output.

@hkhalil-akamai Could you elaborate on what you mean by this? Wouldn't the commands run in order -manager tests would run first and ui tests would run second? That's what I was seeing when I ran it that way, at least.

Using separate commands is okay with me, though - I do just eventually want a way to run all the tests from the root.

bnussman-akamai

General setup looks good!

I do think our themes should move to this package, but I imagine that requires some significant changes. We can do this as soon as it's feasible

bnussman-akamai · 2024-10-09T14:40:19Z

.github/workflows/ci.yml

Can we add a job that typechecks the new UI package?

Added. Side note: why don't we typecheck the api-v4 and validation packages?

The build steps for api-v4 and validation (build-validation, build-sdk) inherently type-check because they run tsc during the build process

packages/ui/package.json

…t component

mjac0bs · 2024-10-10T14:41:10Z

Test failures in CI are not related to this PR; merging this on Hussain's behalf.

cypress · 2024-10-10T15:11:16Z

Cloud Manager E2E Run #6655

Run Properties: Failed #6655 • dbf2e0c067: upcoming: [M3-8638] - Introduce the new `(at)linode/ui` package (#11057)

Project	`Cloud Manager E2E`
Run status	`Failed #6655`
Run duration	`27m 43s`
Commit	dbf2e0c067: upcoming: [M3-8638] - Introduce the new `(at)linode/ui` package (#11057)
Committer	`Hussain Khalil`
View all properties for this run ↗︎

Test results
Failures	`1`
Flaky	`2`
Pending	`2`
Skipped	`0`
Passing	`429`

Tests for review

cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts • 1 failed test

View Output Video

Test		Artifacts
Placement Group deletion > can delete with Linodes assigned when unexpected error show up and retry		`Screenshots` `Video`

clone-linode.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
clone linode > can clone a Linode from Linode details page		`Screenshots` `Video`

rebuild-linode.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
rebuild linode > cannot rebuild a provisioning linode		`Screenshots` `Video`

Introduce the new @linode/ui package

d381228

hkhalil-akamai added Work in Progress Modularization labels Oct 7, 2024

hkhalil-akamai self-assigned this Oct 7, 2024

bnussman-akamai reviewed Oct 7, 2024

View reviewed changes

packages/ui/package.json Outdated Show resolved Hide resolved

packages/ui/src/components/BetaChip/BetaChip.test.tsx Show resolved Hide resolved

hkhalil-akamai added 2 commits October 8, 2024 14:06

Feedback @bnussman-akamai

ffdafaf

Move existing usages of to new package

df0f1cd

hkhalil-akamai commented Oct 8, 2024

View reviewed changes

packages/ui/.eslintrc.json Outdated

Copy link

Contributor Author

hkhalil-akamai Oct 8, 2024

Choose a reason for hiding this comment

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

Copied from packages/api-v4/.eslintrc.json

hkhalil-akamai changed the title ~~WIP: Introduce the new @linode/ui package~~ WIP: Introduce the new (at)linode/ui package Oct 8, 2024

hkhalil-akamai commented Oct 8, 2024

View reviewed changes

packages/ui/.prettierrc Outdated

Copy link

Contributor Author

hkhalil-akamai Oct 8, 2024

Choose a reason for hiding this comment

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

Copied from packages/api-v4/.prettierrc

hkhalil-akamai commented Oct 8, 2024

View reviewed changes

Tweaks

ea34d27

hkhalil-akamai marked this pull request as ready for review October 8, 2024 19:01

hkhalil-akamai requested a review from a team as a code owner October 8, 2024 19:01

hkhalil-akamai requested review from mjac0bs and bnussman-akamai and removed request for a team October 8, 2024 19:01

hkhalil-akamai added Ready for Review and removed Work in Progress labels Oct 8, 2024

hkhalil-akamai requested a review from jaalah-akamai October 8, 2024 19:01

hkhalil-akamai changed the title ~~WIP: Introduce the new (at)linode/ui package~~ Introduce the new (at)linode/ui package Oct 8, 2024

hkhalil-akamai changed the title ~~Introduce the new (at)linode/ui package~~ upcoming: [M3-8638] - Introduce the new (at)linode/ui package Oct 8, 2024

jaalah-akamai reviewed Oct 8, 2024

View reviewed changes

jaalah-akamai approved these changes Oct 8, 2024

View reviewed changes

jaalah-akamai added the Add'tl Approval Needed Waiting on another approval! label Oct 8, 2024

Merge remote-tracking branch 'upstream/develop' into M3-8638-ui-package

b525315

mjac0bs reviewed Oct 8, 2024

View reviewed changes

bnussman-akamai approved these changes Oct 9, 2024

View reviewed changes

Merge remote-tracking branch 'upstream/develop' into M3-8638-ui-package

f498604

hkhalil-akamai added 4 commits October 9, 2024 13:44

Fix unnecessary prop on component

07e4450

Add CHANGELOG.md for UI package

7385010

Added changeset: new @linode/ui package with BetaChip as the firs…

f805147

…t component

Add UI typechecking step to ci.yml

45078a5

hkhalil-akamai force-pushed the M3-8638-ui-package branch from e23a7fa to 45078a5 Compare October 9, 2024 19:43

hkhalil-akamai requested a review from mjac0bs October 9, 2024 19:56

mjac0bs approved these changes Oct 9, 2024

View reviewed changes

mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! Ready for Review labels Oct 9, 2024

mjac0bs merged commit dbf2e0c into linode:develop Oct 10, 2024
22 of 23 checks passed

mjac0bs mentioned this pull request Oct 10, 2024

Release v1.130.0 - release → staging #11087

Merged

hkhalil-akamai deleted the M3-8638-ui-package branch October 11, 2024 12:20

dwiley-akamai mentioned this pull request Oct 11, 2024

refactor: [M3-8640] – Move Theme layer from manager package to ui package #11092

Merged

12 tasks

mjac0bs mentioned this pull request Oct 14, 2024

Release v1.130.0- staging → master #11095

Merged

hkhalil-akamai mentioned this pull request Oct 29, 2024

test: [M3-8809] - Add vitest workspace configuration #11184

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upcoming: [M3-8638] - Introduce the new `(at)linode/ui` package #11057

upcoming: [M3-8638] - Introduce the new `(at)linode/ui` package #11057

hkhalil-akamai commented Oct 7, 2024 •

edited

Loading

bnussman-akamai left a comment

hkhalil-akamai Oct 8, 2024

hkhalil-akamai Oct 8, 2024

hkhalil-akamai Oct 8, 2024 •

edited

Loading

jaalah-akamai Oct 8, 2024

hkhalil-akamai Oct 9, 2024

jaalah-akamai left a comment •

edited

Loading

github-actions bot commented Oct 8, 2024 •

edited

Loading

mjac0bs left a comment •

edited

Loading

mjac0bs Oct 8, 2024 •

edited

Loading

hkhalil-akamai Oct 9, 2024

bnussman-akamai Oct 9, 2024 •

edited

Loading

mjac0bs Oct 9, 2024

bnussman-akamai left a comment

bnussman-akamai Oct 9, 2024

hkhalil-akamai Oct 9, 2024 •

edited

Loading

bnussman-akamai Oct 9, 2024

mjac0bs commented Oct 10, 2024

cypress bot commented Oct 10, 2024

upcoming: [M3-8638] - Introduce the new (at)linode/ui package #11057

upcoming: [M3-8638] - Introduce the new (at)linode/ui package #11057

Conversation

hkhalil-akamai commented Oct 7, 2024 • edited Loading

Description 📝

Changes 🔄

How to test 🧪

bnussman-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hkhalil-akamai Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaalah-akamai left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Oct 8, 2024 • edited Loading

mjac0bs left a comment • edited Loading

Choose a reason for hiding this comment

mjac0bs Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hkhalil-akamai Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjac0bs commented Oct 10, 2024

cypress bot commented Oct 10, 2024

Tests for review

upcoming: [M3-8638] - Introduce the new `(at)linode/ui` package #11057

upcoming: [M3-8638] - Introduce the new `(at)linode/ui` package #11057

hkhalil-akamai commented Oct 7, 2024 •

edited

Loading

hkhalil-akamai Oct 8, 2024 •

edited

Loading

jaalah-akamai left a comment •

edited

Loading

github-actions bot commented Oct 8, 2024 •

edited

Loading

mjac0bs left a comment •

edited

Loading

mjac0bs Oct 8, 2024 •

edited

Loading

bnussman-akamai Oct 9, 2024 •

edited

Loading

hkhalil-akamai Oct 9, 2024 •

edited

Loading