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

feat(core): flatten structure of core package #140

Merged
merged 11 commits into from
Jul 15, 2019
Merged

Conversation

piofinn
Copy link
Contributor

@piofinn piofinn commented Jul 11, 2019

affects: @fremtind/jkl-accordion, @fremtind/jkl-bullet-list, @fremtind/jkl-button,
@fremtind/jkl-card, @fremtind/jkl-checkbox, @fremtind/jkl-core, @fremtind/jkl-divider-line,
@fremtind/jkl-dropdown, @fremtind/jkl-footer, @fremtind/jkl-grid, @fremtind/jkl-header,
@fremtind/jkl-logo, @fremtind/jkl-radio-button, @fremtind/jkl-text-input

📥 Proposed changes

Flatten structure of core package as proposed in issue #139

☑️ Submission checklist

  • I have read the CONTRIBUTING doc
  • yarn build works locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

💬 Further comments

Fixes #139

affects: @fremtind/jkl-accordion, @fremtind/jkl-bullet-list, @fremtind/jkl-button,
@fremtind/jkl-card, @fremtind/jkl-checkbox, @fremtind/jkl-core, @fremtind/jkl-divider-line,
@fremtind/jkl-dropdown, @fremtind/jkl-footer, @fremtind/jkl-grid, @fremtind/jkl-header,
@fremtind/jkl-logo, @fremtind/jkl-radio-button, @fremtind/jkl-text-input

also change all imports accordingly in other packages
@piofinn piofinn requested review from Saegrov and lfbergee July 11, 2019 09:38
Copy link
Contributor

@lfbergee lfbergee left a comment

Choose a reason for hiding this comment

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

I think this is an improvement. We should clear the PRs before taking on this, as this is a breaking change for the entire project.

Also checkout the _index.scss file instead of _variables.scss and _mixin.scss

packages/bullet-list/example/index.scss Show resolved Hide resolved
packages/core/package.json Outdated Show resolved Hide resolved
packages/core/package.json Outdated Show resolved Hide resolved
packages/radio-button/radio-button.scss Outdated Show resolved Hide resolved
@lfbergee
Copy link
Contributor

lfbergee commented Jul 11, 2019

Also cpx, ncp depdendencies can be deleted

@piofinn
Copy link
Contributor Author

piofinn commented Jul 11, 2019

Also checkout the _index.scss file instead of _variables.scss and _mixin.scss

Not possible yet in node-sass as of yet (new feature in LibSass from May)

We should clear the PRs before taking on this, as this is a breaking change for the entire project.

Yep. All existing packages have their imports updated here, but we should consolidate all the incoming changes as well.

@piofinn piofinn requested a review from lfbergee July 15, 2019 08:55
@piofinn piofinn merged commit a18fd73 into master Jul 15, 2019
@piofinn piofinn deleted the refactor-core-0 branch July 15, 2019 10:23
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.

Refactor core package
2 participants