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(data-table): add data-table to sass modules style package #8844

Merged
merged 11 commits into from
Jun 14, 2021

Conversation

tay1orjones
Copy link
Member

Refs #8555

Begins work on bringing DataTable to the @carbon/styles package

Changelog

New

  • include refactored core and partial files added to /styles, including a test

Testing / Reviewing

Test is failing for me locally, but I'm not sure why. 🤔 Any suggestions @joshblack?

Undefined mixin.
       ╷
    11 │ @include data-table.data-table;

@tay1orjones tay1orjones requested a review from a team as a code owner June 4, 2021 21:48
@netlify
Copy link

netlify bot commented Jun 4, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 0d1cab6

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/60ba9fab1f79ec00086ffd91

😎 Browse the preview: https://deploy-preview-8844--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Jun 4, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 9ac56b9

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/60c7a618372c4b0007507d91

😎 Browse the preview: https://deploy-preview-8844--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Jun 4, 2021

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 0d1cab6

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/60ba9fab2c64260007df4802

😎 Browse the preview: https://deploy-preview-8844--carbon-components-react.netlify.app

@netlify
Copy link

netlify bot commented Jun 4, 2021

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 9ac56b9

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/60c7a618073b4200081066b7

😎 Browse the preview: https://deploy-preview-8844--carbon-components-react.netlify.app

@tay1orjones tay1orjones requested a review from a team as a code owner June 10, 2021 15:46
@tay1orjones
Copy link
Member Author

tay1orjones commented Jun 10, 2021

@joshblack Thanks for the review! It prompted me to reorganize this quite a bit.

I removed _data-table-core.scss and instead moved these core styles to be within _data-table.scss. The feature modules (action, expandable, skeleton, sort) moved to their own folder, each with an _index.scss. I kept the mixin names the same. You can see in the test file, but usage would look like:

@use '../data-table';
@use '../data-table/skeleton';

.my-datatable {
  @include data-table();
}

.my-datatable-skeleton {
  @include data-table-skeleton();
}

The goal being that users can bring in only the core styles without having to bring in the feature files if they don't need them. I did modify packages/styles/scss/components/_index.scss to import each of the feature files though so it should all be included in the centralized/main bundle. Tests for all this are updated and should be passing.

@kodiakhq kodiakhq bot merged commit 668ba8e into carbon-design-system:main Jun 14, 2021
@tay1orjones tay1orjones mentioned this pull request Jun 15, 2021
48 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants