PD-381: Port MongoDB Charts Pipeline summary component to LeafyGreen (CHARTS-2864) #191

matt-d-rat · 2019-11-08T04:01:16Z

✍️ Proposed changes

Hey LeafyGreen team! This is my first PR to LeafyGreen, and also the first time I have ever used TypeScript and Emotion so go easy on me 😄.

This PR ports the Pipeline summary component from MongoDB Charts to LeafyGreen. The original implementation of this component can be found here. The port involved converting the original implementation from JavaScript to TypeScript, as well as porting the tests from Mocha / Enzyme to Jest / Testing Library.

Additionally I fixed a TypeScript build error in MenuItem that was preventing me from successfully building the project ~~(the title prop wasn't defined on the interface)~~ (removed the deprecated title prop from the propTypes).

I have also upgraded jest-dom to version 4, the breaking change of which required me to update any imports of the library from jest-dom to @testing-library/jest-dom in order to fix a number of build errors I was getting in other packages.

🎟 Jira ticket: PD-381 / CHARTS-2864

🛠 Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

✅ Checklist

I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)
I have run yarn changeset and documented my changes

💬 Further comments

Screencast

Some things to call out about the implementation:

In order to test the behaviour of the Stage component I had to mock out the browser IntersectionObserver API. I did this via the implementation found in pipeline/src/mocks/IntersectionObserver.ts though I am not sure if this is the best place for this to live?
The style definitions for the Stage and Counter are essentially identical, so I tried to convert them the best way that I could from Less to Emotion via helper functions. Not sure if this is the best approach or whether there are alternatives?
I was unable to find a way to test the behaviour of the useMutationObserver used by the Pipeline component. I attempted to use the waitForDomChange from @testing-library/dom, but the tests always timed out. Not sure if this is a problem with the test that I was writing, or with the @testing-library/dom dependency itself.

bruugey

👋 @matt-d-rat, thank you so much for this PR! Appreciate all the work that went into this component, and excited to have it in LeafyGreen! I left a few comments, mostly nits and things specific to our process within Leafy, but this is really awesome.

packages/menu/src/MenuItem.tsx

packages/modal/src/Modal.spec.tsx

packages/pipeline/package.json

packages/pipeline/src/Counter.tsx

packages/pipeline/README.md

packages/pipeline/package.json

packages/pipeline/src/Stage.tsx

DesignerDave

Hey @matt-d-rat!

Thanks again for the contribution! I did a first-pass here (largely changes for consistency with other components). We'll look into documenting more of these patterns for future contributions.

As a note, we're usually extremely conservative when adding any package to the repository. It would be awesome if we could reduce the number of brand new packages we're introducing here if possible, since 3 feels like a bit much.

packages/pipeline/src/Counter.tsx

packages/pipeline/src/Pipeline.story.tsx

packages/pipeline/src/utils/getPipelineCounterTooltip.ts

packages/pipeline/src/utils/isStageElement.spec.ts

packages/pipeline/src/Stage.tsx

hswolff

Very small initial review...I need to go deeper on the code still. Stand by for a more thorough review sometime tomorrow.

jest.config.js

packages/pipeline/src/mocks/IntersectionObserver.ts

packages/pipeline/src/Counter.tsx

hswolff

Ok! This is my thorough review! Very exciting PR! Thank you so much for contributing!

packages/pipeline/src/index.ts

packages/pipeline/src/Stage.tsx

packages/pipeline/src/Counter.tsx

packages/pipeline/src/Stage.spec.tsx

packages/pipeline/src/Pipeline.tsx

packages/pipeline/src/Stage.tsx

packages/pipeline/src/Pipeline.tsx

matt-d-rat · 2019-12-03T01:55:15Z

@hswolff @brookescarlett @DesignerDave The build is failing for this branch due to:

Exit code 137 - Out of memory

Not sure how to resolve as I don't believe I have the appropriate permissions in CircleCI project to increase the memory of the containers. The build and tests pass when I run them locally, could you please assist me in resolving this issue?

~~See for more info: https://support.circleci.com/hc/en-us/articles/115014359648-Exit-code-137-Out-of-memory~~

Resolved.

hswolff · 2019-12-03T01:56:32Z

Try merging in master. We updated the node version we use on circleCI which seems to help with the memory usage.

matt-d-rat · 2019-12-03T05:19:55Z

Try merging in master. We updated the node version we use on circleCI which seems to help with the memory usage.

~~This branch is up-to-date with current master. The build continues to fail with the same error.~~

Resolved.

packages/pipeline/src/utils/isElementOverflowed.ts

hswolff · 2019-12-03T15:41:46Z

Ugh.... yeah....I wonder if we added a new build:ci command that turns off parallel that would help? Mind trying that?

bruugey · 2019-12-03T18:24:54Z

Also, don't forget to run yarn changeset documenting your changes before merging :)

matt-d-rat · 2019-12-04T05:12:23Z

Ugh.... yeah....I wonder if we added a new build:ci command that turns off parallel that would help? Mind trying that?

I updated the CircleCI .config.yaml in 79c7d43 to turn off the --parallel flag.

.gitignore

packages/pipeline/src/Pipeline.tsx

…reen UI

…. Updated import references across packages.

This reverts commit 3cecf183e35c1fb763da841d95a650aef82c7572.

…ed defaults for functions)

…ort from index.ts)

….gitignore

…r tree shaking

…enAsPiplineStages

…ed util function

…der so it can be re-used with other packages. Update import path for the Stage component tests.

…s memory with Error 137.

…ocks folder so it can be re-used with other packages. Update import path for the Stage component tests." This reverts commit 50f05c10698061008fd1c9d463320124f320505e.

… global .gitignore)

…ted children

hswolff

I dig it! Great work @matt-d-rat!!

matt-d-rat · 2019-12-18T04:46:26Z

Thank you for the approval @hswolff . If I could also get an approval from @brookescarlett or @DesignerDave that would be great.

Just to confirm the process, I would:

Run yarn changeset (which I assume generates some sort of change log?)
Merge to master

After which I assume someone in the LeafyGreen team will handle the release to npm?

bruugey

Yes, be sure to run yarn changeset before merging and then we'll handle publishing from there. Thank you again for your PR 😄

DesignerDave

LGTM! It would be great if we could get a Sketch symbol for this from Dave Hastwell as well, so it's usable outside of Charts 😁

matt-d-rat · 2019-12-18T21:57:41Z

LGTM! It would be great if we could get a Sketch symbol for this from Dave Hastwell as well, so it's usable outside of Charts 😁

cc @dhastwell

I believe one does exist, though he may need to adjust the height of the symbol to correspond to the changes in this PR (I aligned the height of the Pipeline to match the heights of the LeafyGreen buttons at all sizes). I can work with him on that.

dhastwell · 2019-12-20T00:30:36Z

@DesignerDave @matt-d-rat After I make those adjustments, I will happily add this component to our LeafyGreen Sketch library.

…(CHARTS-2864) (#191) * feat: Port Pipeline and Stage component from MongoDB Charts to LeafyGreen UI * chore: Upgrade jest-dom to latest version (@testing-library/jest-dom). Updated import references across packages. * chore: Add typings for external react-merge-refs dependency * chore: Move IntersectionObserver mock to pipeline src * fix: Missing title prop from MenuItemProps typescript interface * Revert "fix: Missing title prop from MenuItemProps typescript interface" This reverts commit 3cecf183e35c1fb763da841d95a650aef82c7572. * fix: Remove deprecated title prop from MenuItem * chore: Pipeline package version set to v0.9.0. Updated all dependencies of Pipeline component to latest versions * chore: Fix incorrect references in JSDoc * chore: PR feedback (deconstruct style, export interface, remove named exports) * chore: PR feedback (prefer flex-grow, flex-shrink, flex-basis over flex shorthand) * chore: PR feedback (Add data attributes using createDataProp) * chore: PR feedback (default util function exports. Remove named exports) * chore: PR feedback (prefer native Array.flatMap instead of lodash method) * chore: PR feedback (update getPupelineCounterTooltip util. Prefer native methods over lodash/fp) * chore: PR feedback (rename isStageElement.spec.ts -> packages/pipeline/src/utils/isStageElement.spec.tsx and use JSX instead of createElement) * chore: PR feedback (set default values in function params for Stage / Counter component styles) * fix: getPipelineCounterTooltip to use lodash flatMap, as native flatMap is not a function in testing environment * fix: Failing test in Logo due to upgrade of jest-dom to testing-library/jest-dom * fix: Trigger observer side effects when Pipeline childList updates. * chore: PR feedback (remove React defaultProps in favour of destructured defaults for functions) * chore: Resolve yarn.lock conflict following rebase * chore: PR feedback - provide one way of importing (remove default export from index.ts) * chore: PR feedback - fix spelling mistakes. Add .vscode directory to .gitignore * chore: PR feedback - prefer ? syntax for optional types * chore: PR feedback - update lodash import to use subpackage for better tree shaking * chore: PR feedback - don't export StateForStyles interface. Unnecessary * chore: PR feedback - don't export Props interface. Unnecessary * chore: PR feedback - rename _children to a more semantic name: childrenAsPiplineStages * chore: PR feedback - improve the description of the isElementOverflowed util function * chore: PR feedback - move IntersectionObserver mock to root mocks folder so it can be re-used with other packages. Update import path for the Stage component tests. * fix: Remove --parallel flag from yarn build command. CircleCI exhausts memory with Error 137. * Revert "chore: PR feedback - move IntersectionObserver mock to root mocks folder so it can be re-used with other packages. Update import path for the Stage component tests." This reverts commit 50f05c10698061008fd1c9d463320124f320505e. * chore: PR feedback. Remove .vscode .gitignore entry (prefer adding to global .gitignore) * chore: PR feedback - Remove Variant feature from Pipeline and associated children * chore: Generate change logs

* VerticalStepper takes actions prop instead of primaryButtonProps and secondaryButtonProps * ActivationSteps uses updated VerticalStepper API * Changeset * Address feedback

…e to leafygreen-ui (#3165) * LG-4333: VerticalStepper (#163) * wip * wip, transitions * image * break out step icon component * tabindex * line color * fix story * status to state * padding * style cleanup * style updates * consts * add tests * README * changeset * types and tests * cleanup * cleanup * lint * cleanup again * some feedback * update icon styles * inert and tabindex * use descedants * remove extra div * update package.json * pass isCompleted to StepIcon * cleanup * feedback * Version Packages (#169) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Fix naming of lg-private packages (#183) * Fix lg-private packages naming * Changeset * LG-4395: feature walls major release (#186) * LG-4395: vertical stepper design QA * LG-4395: feature walls design QA * Changeset * Version Packages (#184) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Use actions slot prop for vertical stepper and activation steps (#191) * VerticalStepper takes actions prop instead of primaryButtonProps and secondaryButtonProps * ActivationSteps uses updated VerticalStepper API * Changeset * Address feedback * Version Packages (#190) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * LG-4412: all sections render in card UI (#194) * Update Section styling and add optional renderInCard prop * FeatureOverview renders in card, handles children container, and removes custom storybook decorator * Templates always renders in card and export type Template * UseCases always renders in card * ActivationSteps type updates and remove storybook decorator * Export types for ActivationStep, Template, and UseCase * Changeset * LG-4413: vertical stepper always renders description and media (#195) * VerticalStepper always renders description and media * Changeset * Cleanup * Version Packages (#197) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * LG-4670: feature walls component resizing (#294) * Update vertical stepper button layout on smaller breakpoints * Fix resizing in AccordionPanel * Fix resizing in ExpandableGrid * Feedback * height is always available * Version Packages (#292) * Version Packages * Version next @lg-private/vertical-stepper and update changelogs --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Stephen Lee <stephen.lee@mongodb.com> * Updates Popover/Menu/Tooltip to TopLayer LG-4746 (#337) * bump other packages * major popover updates * bump provider * bump global provider * bump cli * rm popoverZIndex * Create forty-days-rest.md * Delete .eslintignore * update lg to latest major * Removes prop-types, and bump LG packages * lint * Update .eslintrc.js * update storybook @lg-tools/storybook-utils * lint * Version Packages (#343) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * PNPM + Storybook 8 (private) (#426) * pnpm import * Delete yarn.lock * workflows * yarn -> pnpm docs * workspace:^ * add missing global packages * @storybook/test * story ts fixes * Adds missing peers * rm lg.json * bump base lg * fix product icon builds * fix sb * bump peer deps * rm pnpm/action-setup@v4 * Revert "rm pnpm/action-setup@v4" This reverts commit 14198eb258e958ddaa0e480fee17cfddec5ce6fd. * Update pnpm-lock.yaml * apk * apk all * wget * restore pnpm * specific pnpm action * workspace * lint fix * tools @ latest * minor lint errors * tools build dev deps * typescript * tsconfig * package exports * ts fixes * Create rollup.config.mjs * lint * checkout main src * ts fixes * lint * isolated async tests * Update CloudNav.analytics.spec.tsx * skip mongonav tests * feature walls tests * Create rollup.config.mjs * lint fix * Update pr.yml * Update release.yml * cache/restore * chromatic + pnpm * update cache restore paths * cache save path * addon 0.5.2-next.0 * addon 0.5.2-next.2 * chore(vertical-stepper): move from @lg-private to @leafygreen-ui scope * chore(vertical-stepper): disable snapshot of LiveExample story --------- Co-authored-by: Shaneeza <shaneeza.ali@mongodb.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Adam Thompson <2414030+TheSonOfThomp@users.noreply.github.com>

matt-d-rat requested review from hswolff, DesignerDave and bruugey November 8, 2019 04:01

matt-d-rat self-assigned this Nov 8, 2019

bruugey reviewed Nov 12, 2019

View reviewed changes

matt-d-rat force-pushed the feat/port-charts-pipeline-component branch from bb9fced to 090a6f0 Compare November 12, 2019 21:56

matt-d-rat requested a review from bruugey November 13, 2019 23:13

matt-d-rat force-pushed the feat/port-charts-pipeline-component branch from 4fb3d4b to e8b91b5 Compare November 14, 2019 22:23

DesignerDave requested changes Nov 18, 2019

View reviewed changes

matt-d-rat force-pushed the feat/port-charts-pipeline-component branch from e8b91b5 to b826468 Compare November 21, 2019 03:31

hswolff reviewed Nov 21, 2019

View reviewed changes

jest.config.js Show resolved Hide resolved

packages/pipeline/src/mocks/IntersectionObserver.ts Outdated Show resolved Hide resolved

packages/pipeline/src/Counter.tsx Outdated Show resolved Hide resolved

hswolff requested changes Nov 22, 2019

View reviewed changes

matt-d-rat force-pushed the feat/port-charts-pipeline-component branch from ed26972 to ffa74d1 Compare December 3, 2019 00:28

hswolff requested changes Dec 3, 2019

View reviewed changes

packages/pipeline/src/utils/isElementOverflowed.ts Outdated Show resolved Hide resolved

matt-d-rat requested review from hswolff and DesignerDave December 4, 2019 05:12

DesignerDave reviewed Dec 12, 2019

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

packages/pipeline/src/Pipeline.tsx Outdated Show resolved Hide resolved

packages/pipeline/src/Pipeline.tsx Outdated Show resolved Hide resolved

matt-d-rat added 7 commits December 16, 2019 12:01

feat: Port Pipeline and Stage component from MongoDB Charts to LeafyG…

368604f

…reen UI

chore: Upgrade jest-dom to latest version (@testing-library/jest-dom)…

234f9c4

…. Updated import references across packages.

chore: Add typings for external react-merge-refs dependency

f7c26ad

chore: Move IntersectionObserver mock to pipeline src

05110b6

fix: Missing title prop from MenuItemProps typescript interface

e50c168

Revert "fix: Missing title prop from MenuItemProps typescript interface"

fcba14e

This reverts commit 3cecf183e35c1fb763da841d95a650aef82c7572.

fix: Remove deprecated title prop from MenuItem

6665587

matt-d-rat added 16 commits December 16, 2019 12:02

fix: Trigger observer side effects when Pipeline childList updates.

c1660c3

chore: PR feedback (remove React defaultProps in favour of destructur…

6d0ee4f

…ed defaults for functions)

chore: Resolve yarn.lock conflict following rebase

a3b2df5

chore: PR feedback - provide one way of importing (remove default exp…

4f4a0e9

…ort from index.ts)

chore: PR feedback - fix spelling mistakes. Add .vscode directory to …

b413193

….gitignore

chore: PR feedback - prefer ? syntax for optional types

4295dad

chore: PR feedback - update lodash import to use subpackage for bette…

d2a3cfc

…r tree shaking

chore: PR feedback - don't export StateForStyles interface. Unnecessary

d15f443

chore: PR feedback - don't export Props interface. Unnecessary

beadd1d

chore: PR feedback - rename _children to a more semantic name: childr…

c863adc

…enAsPiplineStages

chore: PR feedback - improve the description of the isElementOverflow…

416a209

…ed util function

chore: PR feedback - move IntersectionObserver mock to root mocks fol…

b178135

…der so it can be re-used with other packages. Update import path for the Stage component tests.

fix: Remove --parallel flag from yarn build command. CircleCI exhaust…

cccf6d1

…s memory with Error 137.

Revert "chore: PR feedback - move IntersectionObserver mock to root m…

b054431

…ocks folder so it can be re-used with other packages. Update import path for the Stage component tests." This reverts commit 50f05c10698061008fd1c9d463320124f320505e.

chore: PR feedback. Remove .vscode .gitignore entry (prefer adding to…

0cf5485

… global .gitignore)

chore: PR feedback - Remove Variant feature from Pipeline and associa…

b6c1bc0

…ted children

matt-d-rat force-pushed the feat/port-charts-pipeline-component branch from 79c7d43 to b6c1bc0 Compare December 16, 2019 02:52

matt-d-rat requested a review from DesignerDave December 16, 2019 03:16

hswolff approved these changes Dec 17, 2019

View reviewed changes

bruugey approved these changes Dec 18, 2019

View reviewed changes

DesignerDave approved these changes Dec 18, 2019

View reviewed changes

chore: Generate change logs

c743f4c

matt-d-rat merged commit 31f6bfd into master Dec 18, 2019

matt-d-rat deleted the feat/port-charts-pipeline-component branch December 18, 2019 22:27

PD-381: Port MongoDB Charts Pipeline summary component to LeafyGreen (CHARTS-2864) #191

PD-381: Port MongoDB Charts Pipeline summary component to LeafyGreen (CHARTS-2864) #191

Uh oh!

Conversation

matt-d-rat commented Nov 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✍️ Proposed changes

🛠 Types of changes

✅ Checklist

💬 Further comments

Screencast

Uh oh!

bruugey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DesignerDave left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hswolff left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hswolff left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matt-d-rat commented Dec 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hswolff commented Dec 3, 2019

Uh oh!

matt-d-rat commented Dec 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

hswolff commented Dec 3, 2019

Uh oh!

bruugey commented Dec 3, 2019

Uh oh!

matt-d-rat commented Dec 4, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hswolff left a comment

Choose a reason for hiding this comment

Uh oh!

matt-d-rat commented Dec 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bruugey left a comment

Choose a reason for hiding this comment

matt-d-rat commented Nov 8, 2019 •

edited

Loading

matt-d-rat commented Dec 3, 2019 •

edited

Loading

matt-d-rat commented Dec 3, 2019 •

edited

Loading

matt-d-rat commented Dec 18, 2019 •

edited

Loading