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.

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

Component Classification #102

Merged
merged 12 commits into from Mar 15, 2021

Conversation

alexgetty
Copy link
Contributor

@alexgetty alexgetty commented Jan 27, 2021

This PR incorporates Issue #84, which reclassifies numerous Atoms as Molecules based on their HTML structure. All Atoms with more than 1 HTML element, with the exception of icons and wrappers for icon positioning, have been moved to the Molecules layer. Additionally, there were a handful of icon-variant components that were merged into their base component with the ability to optionally add an icon prop.

  • Related GitHub issue(s) linked in PR description
  • Destination branch merged, built and tested with your changes
  • Code formatted and follows best practices and patterns
  • Code builds cleanly (no additional warnings or errors)
  • Manually tested
  • Automated tests are passing
  • No decreases in automated test coverage
  • Documentation updated (readme, docs, comments, etc.)
  • Localization: No hard-coded error messages in code files (minimally in string constants)
  • New component and/ or properties? Make sure to update storybook

@codecov
Copy link

codecov bot commented Jan 27, 2021

Codecov Report

Merging #102 (feabdd5) into main (e4e3010) will increase coverage by 1.84%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #102      +/-   ##
==========================================
+ Coverage   38.76%   40.60%   +1.84%     
==========================================
  Files          51       46       -5     
  Lines         908      857      -51     
  Branches      206      191      -15     
==========================================
- Hits          352      348       -4     
+ Misses        556      509      -47     
Impacted Files Coverage Δ
src/molecules/checkbox-button/checkbox-button.tsx 100.00% <ø> (ø)
src/molecules/checkbox-input/checkbox-input.tsx 100.00% <ø> (ø)
src/molecules/form-fields/checkbox-form-field.tsx 100.00% <ø> (ø)
src/molecules/form-fields/select-form-field.tsx 100.00% <ø> (ø)
src/molecules/progress-bar/progress-bar.tsx 100.00% <ø> (ø)
src/molecules/select/select.tsx 100.00% <ø> (ø)
src/molecules/toasts/toast-templates.tsx 100.00% <ø> (ø)
src/utilities/toast-manager.ts 100.00% <ø> (ø)
src/atoms/anchors/anchor.tsx 100.00% <100.00%> (ø)
src/atoms/buttons/button.tsx 100.00% <100.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4e3010...9237537. Read the comment docs.

Copy link
Contributor

@brandongregoryscott brandongregoryscott left a comment

Choose a reason for hiding this comment

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

Left some comments for cleanup, but thanks for getting this together 👍

src/atoms/anchors/anchor.stories.tsx Outdated Show resolved Hide resolved
src/atoms/anchors/anchor.stories.tsx Outdated Show resolved Hide resolved
src/atoms/anchors/anchor.tsx Outdated Show resolved Hide resolved
src/atoms/anchors/anchor.tsx Outdated Show resolved Hide resolved
src/atoms/forms/text-input.test.tsx Outdated Show resolved Hide resolved
src/atoms/typography/heading.scss Outdated Show resolved Hide resolved
src/atoms/typography/heading.stories.tsx Outdated Show resolved Hide resolved
src/atoms/typography/heading.tsx Outdated Show resolved Hide resolved
src/molecules/checkbox-button/checkbox-button.tsx Outdated Show resolved Hide resolved
src/molecules/radio-input/radio-input.scss Outdated Show resolved Hide resolved
@brandongregoryscott
Copy link
Contributor

@alexgetty @phess101 Looks like all of my comments have been resolved, but the build is failing. I've pulled down your fork and the same error is occurring for me locally, so it doesn't seem to be an issue with the CI environment. None of the changes were to either of the components referenced (ListBox or DragAndDropListBox), so not sure what's up there. Let me know if you'd like to schedule some pairing time to debug it

@brandongregoryscott
Copy link
Contributor

@phess101 / @alexgetty just a heads up, the docs folder is regenerated when you run a build but shouldn't normally be committed - it'll contain references to your fork/local commits. It's safe to blow away with a git checkout docs/. We regenerate and publish the docs on release. I've already reverted it for this PR, but just a heads up. Thanks for the contribution here!

@brandongregoryscott
Copy link
Contributor

@all-contributors add @phess101 for maintenance

@allcontributors
Copy link
Contributor

@brandongregoryscott

I've put up a pull request to add @phess101! 🎉

@brandongregoryscott
Copy link
Contributor

@all-contributors add @alexgetty for maintenance

@allcontributors
Copy link
Contributor

@brandongregoryscott

I've put up a pull request to add @alexgetty! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants