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

Use plain CSS to eliminate need for Sprockets and Sass building #93

Merged
merged 2 commits into from
Mar 8, 2025

Conversation

GUI
Copy link
Contributor

@GUI GUI commented Mar 7, 2025

I'm not sure if you'd be open to this change, but this tries to simplify usage of this gem with the evolving JS/CSS landscape. So while #92 made the sass compilation more flexible, this still wasn't compatible with certain setups (like vite_rails, since that doesn't use Sprockets), so this tries to simplify things further by eliminating the need for any type of specific asset pipeline solution.

While there might be various other approaches to this, since the CSS/JS dependencies in this app seem actually pretty straightforward, it seemed like the simplest approach might be to just inline the CSS into the default layout so there's no explicit dependencies. A similar gem, Maily, took this same inline approach: markets/maily#48. I also saw a comment from @rromanchuk in #92 with a somewhat similar fork to try and eliminate the need for Sass building steps.

Changes:

  • Compile the Sass files into resulting plain CSS and use those pre-built files so that no Sass build step is needed.
  • Inline the CSS and favicon into the default layout file to simplify compatibility across various different asset solutions. This way it doesn't matter if the app is using Asset Pipeline/Sprockets, Webpacker, Vite, or none of the above.
  • Remove Turbolinks javascript, since the library is deprecated. Embedding applications could choose to add it if desired, but since the functionality was optional, it seems simplest to skip including it directly in the gem.

Applications using the gem could still have options for how to customize things, since apps should still be able to import the application.css or bootstrap.css file into their own build process or Sass files. And similarly, custom layouts could still be used to tailor things further. I was just hoping this might provide an easier compatibility story for the gem out-of-the-box regardless of what asset solution the app is using.

- Compile the Sass files into resulting plain CSS and use those
  pre-built files so that no Sass build step is needed.
- Inline the CSS and favicon into the default layout file to simplify
  compatibility across various different asset solutions. This way it
  doesn't matter if the app is using Asset Pipeline/Sprockets,
  Webpacker, Vite, or none of the above.
- Remove Turbolinks javascript, since the library is deprecated.
  Embedding applications could choose to add it if desired, but since
  the functionality was optional, it seems simplest to skip including it
  directly in the gem.
@glebm
Copy link
Owner

glebm commented Mar 7, 2025

Can you please use CSS variables for what were previously Sass variables? That will preserve the option to do simple customizations, giving an easier migration path.

Setup CSS variables to mimic most of the Sass variables that were
previously in use by the Sass stylesheets.

This introduces some additional variables to capture the various
variations on colors that were based on Sass `lighten()` or `darken()`
calls on the other variables.

The one exception that is no longer a variable is the
`$rep-grid-breakpoint-sm: 768px`, since CSS media queries can't use CSS
variables.
@GUI
Copy link
Contributor Author

GUI commented Mar 8, 2025

@glebm: Great call! I've tried to replicate all of the existing variables in f9b840f As noted in the commit, there's just a couple variations:

  1. There are some new, additional CSS variables to capture the various variations on colors that were based on Sass lighten() or darken() calls on the other variables.
  2. One exception that is no longer a variable is the $rep-grid-breakpoint-sm: 768px, since CSS media queries can't use CSS variables.

I'm open to suggestions on the variable names or there's maybe other ways that some of this could be reworked, I was mainly just aiming for parity with what the Sass was being compiled into.

@glebm glebm merged commit 028f9c2 into glebm:main Mar 8, 2025
2 of 10 checks passed
@glebm
Copy link
Owner

glebm commented Mar 8, 2025

Thanks!

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