-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allow extending Admin's tailwind #5080
Conversation
Codecov Report
@@ Coverage Diff @@
## nebulab/admin #5080 +/- ##
================================================
Coverage ? 88.51%
================================================
Files ? 563
Lines ? 13892
Branches ? 0
================================================
Hits ? 12296
Misses ? 1596
Partials ? 0 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
7b6ee3a
to
99ddff8
Compare
99ddff8
to
293ce97
Compare
I have added three extra commits: Allow overriding SolidusAdmin's configuration file
A Allow overriding SolidusAdmin's Tailwind stylesheetSame for the Tailwind stylesheet. That can be helpful for advanced CSS customizations. We're adding a Create an initializer when installing Solidus adminThat will help users to know what they can configure about the admin interface. For now, tailwind options are the only ones available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking over this, I don't have any strict preferences about how to allow extending tailwind.
All the changes and points you've tackled here make a lot of sense to me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a typo in the first commit ("configuratio")
Fixed, thanks!!
end | ||
|
||
def build_tailwind | ||
rake "solidus_admin:tailwindcss:build" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this task needed during the installation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the compiled CSS is generated in the host application, that's to allow users to extend the admin style, for instance, using other tailwind classes or stylesheets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it! I somehow thought that was already part of the watch task. Do you think it might be a good idea to always run it when the watch task is executed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@waiting-for-dev would it still be possible to ship a compiled CSS that won't require tailwind to be installed for all those not needing to customize?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave more thought to it, and I think it'd be possible, but at the expense of a more complex setup.
We could keep the generation of the CSS for the solidus_admin engine within the development process of it. Then, let's consider a host application that needs to reuse. It should install tailwind, and we could have a different task using the same tailwind config file (the erb template) from the engine but generating the CSS output file to an application path matching the one in solidus_admin.
If we add other engines into the mix, each one should do something similar to what I described for the host application. However, each of them should ship an isolated CSS file, so we should change the solidus_admin manifest to take all of them.
I wonder if it pays off, as now customization is more streamlined for both scenarios. Happy to explore it further if you think that would be a good idea, but I'd prefer to do that on another PR. We can keep it in the back of our minds as a future optimization if we feel the need for it.
293ce97
to
a9aa61c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff!
I left some questions and doubts, but also approving, up to you if it's worth updating this PR or use a follow up PR (if needed).
end | ||
|
||
def build_tailwind | ||
rake "solidus_admin:tailwindcss:build" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@waiting-for-dev would it still be possible to ship a compiled CSS that won't require tailwind to be installed for all those not needing to customize?
a9aa61c
to
5fcd766
Compare
5fcd766
to
401c889
Compare
We namespace all our assets under `solidus_admin` to avoid conflicts. We use an `application.css` manifest. From it, we require the compiled tailwind css, which lives within `app/assets/builds` and is named `tailwind.css`, just like tailwindcss-rails does with host apps.
…sses With this change, the host app and extensions can make use of the tailwind classes defined in the admin. They can also extend the tailwind configuration by adding their own classes. As a requisite for this change, the compiled CSS is now generated in the host app's assets directory, and tailwindcss-rails needs to be an explicit dependency for solidus_admin. The way it works is that we convert the tailwind.config.js file into a `.js.erb` template. We populate the `content` property by reading from a `SolidusAdmin::Config#tailwind_content` setting. Through custom `solidus_admin:tailwindcss:build` and `solidus_admin:tailwindcss:watch` tasks, we generate the actual tailwind configuration file on the fly. By default, the `tailwind_content` setting contains both the admin's and the host app's paths scoped to `solidus_admin` directories, so a host application is not required to do further configuration. On the other side, extensions can manually push new paths on an initializer. As solidus_admin's configuration file requires access to `Rails.root` to create those defaults, the file must be required after the Rails application is created. The solidus_admin's tailwind tasks have been moved to the `lib/tasks` directory so they can be picked by the `bin/rails` command from within the host app. When solidus_admin is installed, tailwind is automatically built and the compiled file is added to the `.gitignore` file. The compilation is also added as an enhancement for the `assets:precompile` task.
In a similar way we were doing for the tailwind configuration file, we're now allowing the host app and extensions to add their own Tailwind CSS. This is done by changing the `application.tailwind.css` file to an ERB template. Both the host application and extensions can add their own stylesheets through the `tailwind_stylesheets` configuration option. During compilation time, the content for those `tailwind_stylesheets` is appended to the dynamically generated CSS file. As adding new styles should be thoroughly thought as per tailwind recomendations, we're not creating a default stylesheet for the host app and, instead, it needs to be manually created. The way we're doing this can be seen as a bit clumsy, but the alternative requires using `postcss-import` and adding all the npm jazz to the mix. There have been some discussion on tailwindcss-rails about this [2], but there's no built-in way as of today. [1] - https://tailwindcss.com/docs/using-with-preprocessors#build-time-imports [2] - rails/tailwindcss-rails#133
We were already allowing the configuration of the scanned paths through `SolidusAdmin::Config::tailwind_content`. We're now also allowing overriding the configuration file when `app/config/solidus_admin/tailwind.config.js.erb` is present. That can be helpful when more advanced configuration is needed, like adding custom plugins or extending the default theme. A `bin/rails tailwind:override` task has been added to help with this.
In the same way that we're allowing overriding Tailwind's configuration file, we're now allowing overriding the Tailwind stylesheet. That can be helpful for advanced CSS customizations. We're adding a `bin/rails tailwind:override_stylesheet` task that copies the default stylesheet to `app/assets/stylesheets/solidus_admin/application.tailwind.css.erb`.
Methods defined within the `namespace` block of a rake task are defined on `Object` and are thus available everywhere. We fix it by extracting them to a module. We're also renaming the `#compile_file` method to `#compile_to_tempfile`.
401c889
to
d941678
Compare
Summary
Allow host app & extensions to make use & extend admin's tailwind classes as well as adding new CSS.
As a requisite for this change, the compiled CSS is now generated in the host app's assets directory, and tailwindcss-rails needs to be an explicit dependency for solidus_admin.
With this change, the host app and extensions can make use of the tailwind classes defined in the admin. They can also extend the tailwind configuration by adding their own classes.
The way it works is that we convert the tailwind.config.js file into a
.js.erb
template. We populate thecontent
property by reading from aSolidusAdmin::Config#tailwind_content
setting. Through customsolidus_admin:tailwindcss:build
andsolidus_admin:tailwindcss:watch
tasks, we generate the actual tailwind configuration file on the fly.By default, the
tailwind_content
setting contains both the admin's and the host app's paths scoped tosolidus_admin
directories, so a host application is not required to do further configuration. On the other side, extensions can manually push new paths on an initializer. As solidus_admin's configuration file requires access toRails.root
to create those defaults, the file must be required after the Rails application is created.The solidus_admin's tailwind tasks have been moved to the
lib/tasks
directory so they can be picked by thebin/rails
command from within the host app.When solidus_admin is installed, tailwind is automatically built and the compiled file is added to the
.gitignore
file. The compilation is also added as an enhancement for theassets:precompile
task.In a similar way, we're now allowing the host app and extensions to add their own Tailwind CSS.
This is done by changing the
application.tailwind.css
file to an ERB template. Both the host application and extensions can add their own stylesheets through thetailwind_stylesheets
configuration option. During compilation time, the content for thosetailwind_stylesheets
is appended to the dynamically generated CSS file.As adding new styles should be thoroughly thought as per tailwind recomendations, we're not creating a default stylesheet for the host app and, instead, it needs to be manually created.
The way we're doing this can be seen as a bit clumsy, but the alternative requires using
postcss-import
and adding all the npm jazz to the mix. There have been some discussion on tailwindcss-rails about this, but there's no built-in way as of today.If we go forward with this proposal, we can add new settings to allow configuring other parts of tailwind's config file, like plugins.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: