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

Add Admin Tailwind build when generating sandbox #5636

Merged

Conversation

spaghetticode
Copy link
Member

@spaghetticode spaghetticode commented Jan 30, 2024

Summary

Fixes #5598

This PR attempts to fix the error couldn't find file 'solidus_admin/tailwind.css' when loading admin pages from the sandbox application.

The error is caused by the absence of the Admin Tailwind CSS build file, which doesn't exist on (local) copies of the project codebase from GitHub.

The fix adds the new flag --build-admin-tailwind to the install process that controls whether to build the CSS file and install the associated rake tasks into the sandbox application. The flag defaults to true, so installing Solidus will build Admin Tailwind CSS and add rake tasks. This can be disabled with rails g solidus:install --no-build-admin-tailwind, see rails g solidus:install --help for more context.

The only effective way to run the server is now via bin/dev.

Also, the main README is updated with the new relevant information.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@spaghetticode spaghetticode self-assigned this Jan 30, 2024
@github-actions github-actions bot added changelog:solidus_core Changes to the solidus_core gem changelog:solidus Changes to the solidus meta-gem changelog:repository Changes to the repository not within any gem labels Jan 30, 2024
@spaghetticode spaghetticode marked this pull request as ready for review January 30, 2024 12:29
@spaghetticode spaghetticode requested a review from a team as a code owner January 30, 2024 12:29
@kennyadsl
Copy link
Member

Thanks @spaghetticode. I don't understand why the build file is not present locally only, or if you use the GH version of Solidus, while it's present if you use the RubyGems version. Can you help me?

@spaghetticode
Copy link
Member Author

@kennyadsl that's because by design we've decided to avoid committing the CSS build file, so it's absent from GH/local copies of the repo.
On the other hand, the file is present on released versions because it's built at release time locally, see #5554, https://github.com/solidusio/solidus/blob/main/tasks/releasing.rake for the customizations we did to the release task, and https://github.com/solidusio/solidus/blob/main/admin/Rakefile#L34 for the admin build task run from the previous file that aliases the Tailwind build task.

@kennyadsl
Copy link
Member

Ah, you are right, got it.

What do you think about building this file during the installation the default option?

  • When you use the official gem and install Solidus for the first time, the result should be identical; if they are not the same, it's still better to rebuild it.
  • When you use a version of the gem that doesn't have the file, it will still work.

What you did works, but I would hide this technical detail to new users.

@spaghetticode spaghetticode force-pushed the spaghetticode/admin-tailwind-sandbox-fix branch from 52e6e5d to 5461604 Compare February 7, 2024 16:55
@github-actions github-actions bot removed the changelog:repository Changes to the repository not within any gem label Feb 7, 2024
@spaghetticode
Copy link
Member Author

@kennyadsl thanks for your comment, I updated the PR description and the code and now the flag is set to true by default so it always builds. Let me know if it works for you, thanks!

This facilitates editing sandbox routes and Procfile when installing
Tailwind.
This way, we can provide the option to (not) build the Tailwind CSS
file and add rake tasks for managing Admin Tailwind assets.

The flag is added to the Solidus installer, then passed along when
installing the new admin.
The preferred (AKA the only way that fully works) way to start the
sanbox is via `bin/dev`. Same thing when installing Solidus from
Github or the local filesystem.
@spaghetticode spaghetticode force-pushed the spaghetticode/admin-tailwind-sandbox-fix branch from 5461604 to 26ed5f7 Compare February 8, 2024 13:40
apply_template_for :authentication, @selected_authentication
apply_template_for :frontend, @selected_frontend
apply_template_for :payment_method, @selected_payment_method
generate "solidus_admin:install #{'--tailwind' if options[:build_admin_tailwind]}"
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the back and forth (and it might not be related to this PR), but at this point I don't understand in which case we don't need to install Tailwind along with the new admin. I mean, in which circumstances build_admin_tailwind will be set to false?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now, never. But if for any reason somebody wants to disable it, we give the possibility to do it with rails g solidus:install --no-build-admin-tailwind, for example.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

👍

@spaghetticode spaghetticode merged commit 656afaa into solidusio:main Feb 20, 2024
12 checks passed
@spaghetticode spaghetticode deleted the spaghetticode/admin-tailwind-sandbox-fix branch February 20, 2024 08:55
@spaghetticode spaghetticode added the backport-v4.3 Backport this pull-request to v4.3 label Feb 28, 2024
Copy link

💔 All backports failed

Status Branch Result
v4.3 Backport failed because of merge conflicts

You might need to backport the following PRs to v4.3:
- Install Solidus Admin after other subcomponents

Manual backport

To create the backport manually run:

backport --pr 5636

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@spaghetticode spaghetticode added backport-v4.3 Backport this pull-request to v4.3 and removed backport-v4.3 Backport this pull-request to v4.3 labels Mar 11, 2024
Copy link

💚 All backports created successfully

Status Branch Result
v4.3

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v4.3 Backport this pull-request to v4.3 changelog:solidus_core Changes to the solidus_core gem changelog:solidus Changes to the solidus meta-gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sandbox does not work in v4.3 and main
3 participants