feat(scss): Optimized SCSS to remove need to separately load @carbon/styles #1652
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… optimize use of globals
…o be individually loaded
nstuyvesant
requested review from
theiliad,
Akshat55 and
a team
as code owners
✅ Deploy Preview for carbon-charts-core ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-charts-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-charts-angular ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Fixed with latest push. |
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problems this solves...
@carbon/chartsand the packages for React, Angular, Svelte, and Vue required that@carbon/styles/css/styles.cssbe loaded IN ADDITION TO the styles for the chart component libraries. This caused the entire stylesheet for@carbon/stylesto be loaded including styles for components not used by@carbon/chartsincreasing download times. It also added extra work to implement@carbon/charts(more moving parts and more documentation).@carbon/styles/css/styles.cssalso applies a CSS reset which messes up Bootstrap and Material Design's CSS reset. That forces developers who do not want body-level@carbon/stylesstyling to use the shadow DOM for components.@carbon/styleswas only needed for the toolbar and its modal (tabular representation). These are overflow-menu, button, modal, and data-table modules in@carbon/stylesthat should have been loaded by their corresponding Carbon Charts SCSS components.@carbon/chartsSCSS used@importwhich is deprecated@carbon/import-oncewas used as a workaround because@importwas used instead of@usefor network diagram components.@use 'something' as *;which makes it difficult to understand where variables originate.@useby other modules.@carbon/stylesandsass(latter to avoid an install prompt by StackBlitz).@carbon/charts-angularexports specified the dist folder for styles.css and styles.min.css but because the dist folder is what's published (rather than the parent), these paths were wrong.How to test this
Because StackBlitz normally loads styles.css from the latest version of
@carbon/charts, please use this StackBlitz example. The file packages/core/dist/styles.css has been copied and pasted into a file in the example then loaded. Please note that the Zoom button is always hidden in StackBlitz examples (not specific to this PR).