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

refactor(ui): switch css to scss #65

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

refactor(ui): switch css to scss #65

wants to merge 3 commits into from

Conversation

jrson83
Copy link
Collaborator

@jrson83 jrson83 commented Sep 14, 2023

Description

Main purpose is to refactor code by replacing CSS with modern modular approach SCSS.

Will result in:

  • cleaner code for development
  • probably reduce output size of final CSS bundle
  • try to reduce number of DOM Nodes for faster rendering
  • properly perfectionize the almost perfect light/dark switch

Roadmap

  • refactor(ui): add scss support
  • refactor(ui): add stylelint & initial scss files
  • refactor(ui): add sass for debug, migrate colors (still wip)
  • next steps ...

WIP Draft

I will use parts of the code of my wip scss library.

@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (8dc19f6) 2.76% compared to head (8b83c23) 2.76%.

Additional details and impacted files
@@          Coverage Diff          @@
##            main     #65   +/-   ##
=====================================
  Coverage   2.76%   2.76%           
=====================================
  Files         44      44           
  Lines       2786    2786           
  Branches      57      57           
=====================================
  Hits          77      77           
  Misses      2666    2666           
  Partials      43      43           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

},
"pnpm": {
"overrides": {
"esbuild": "0.19.2"
Copy link
Owner

@itsjavi itsjavi Sep 15, 2023

Choose a reason for hiding this comment

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

why do we need this override here?

Copy link
Collaborator Author

@jrson83 jrson83 Sep 15, 2023

Choose a reason for hiding this comment

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

We need to overwrite tsup esbuild version v0.18, since the sass plugin using v0.19 where the Plugin type definition changes. This can be removed after tsup updates to esbuild v0.19.

/// @param {Map} $map - The map.
/// @param {Arglist} $keys - The key chain.
/// @return {*} - The desired value.
@function map-deep-get($map, $keys...) {
Copy link
Owner

Choose a reason for hiding this comment

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

why do we need all this setup and utility code? it's more than 1000 lines of code just to add scss support and I am worried about the maintainability, because it adds complexity to the codebase

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't worry and please wait till you see the advantages for storylite users and us when using these few utility classes.

@itsjavi itsjavi added the improvement Enhancements and/or refactorings of existing features label Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Enhancements and/or refactorings of existing features
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

None yet

2 participants