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

Feature: Optimizing SCSS and moving towards a design system... ...eventually 🦄 #139

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

32teeth
Copy link
Member

@32teeth 32teeth commented Oct 16, 2024

Problem

The current SCSS code lacks proper modularization and reusability, leading to potential repetition and complexity. There's a need for improved structure and clarity, specifically in handling CSS variables, transitions, and various visual aspects (such as borders, shadows, radii, etc.). Maintaining consistency across multiple elements and enhancing reusability in the SCSS files will improve maintainability and readability.

Solution

The solution involves introducing a set of reusable SCSS mixins to handle the following:

  1. Transition Variables (mynah-transition-var) - A mixin to handle transitions with customizable properties, duration, and timing functions.
  2. Sizing Variables (mynah-sizing-vars) - A mixin for dynamically generating sizing variables.
  3. CSS Variable Generator (mynah-css-var) - A flexible mixin to assign CSS variables.

The solution also includes a mynah-generate-css-var mixin to dynamically handle CSS variable generation based on different types (prefix, radius, shadow), allowing for consistent and efficient CSS variable management.

Finally, the SCSS variables and collections for colors, syntax, transitions, shadows, and radii have been standardized and modularized using scss-variables, ensuring consistency across the project.

Main Changes:

  • Added reusable mixins for common patterns: borders, spacing, fonts, transitions, and shadows.
  • Modularized SCSS variables ($mynah-collections, $mynah-status-colors, etc.) to improve readability and reuse.
  • Updated @each loops to handle transitions and CSS variable assignments dynamically, improving efficiency.
  • Added standardized SCSS variables for text colors, shadows, radii, and transitions, ensuring consistency.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@32teeth 32teeth requested a review from a team as a code owner October 16, 2024 16:16
@dogusata
Copy link
Collaborator

It seems like a nice improvement, but also at the same time this causes issue on how we expose the css custom properties.

1- Changing the names of the items in the CSS custom properties will require changes in consumers which use the library. Here, the evaluation between profit and loss should be made very carefully. Are we really having benefits by renaming the css custom properties?

2- There is a reason to keep the CSS custom property list as a pure css custom property list. Instead of letting consumer to read the file and understand what variables are available by investigating the SCSS the functions, having a list directly in hand is a wanted approach. Additionally, for intellisense, having direct access to the whole list of css custom properties without additional configurations in IDEs would be more beneficial than having a function which just handles a few custom properties.

We'll hold this PR and go further after investigation.

@32teeth
Copy link
Member Author

32teeth commented Oct 22, 2024

Do we allow users to change SCSS variable or simply reassign them in their local settings CSS?

@dogusata
Copy link
Collaborator

Do we allow users to change SCSS variable or simply reassign them in their local settings CSS?

Actually no. If they're build the whole package, they can change of course. But if they're just consuming the mynah-ui (which is the expected use case for this repo), there is no SCSS variable at all. Only the css custom properties will be available on the runtime. Which, for example being changed on the runtime when the VSCode theme changes. So the css custom property list should be available from some place (when we have a design system like approach it will be easier) which is the _variables.scss file for now.

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.

2 participants