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

Document custom assets #2475

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

Conversation

pablobm
Copy link
Collaborator

@pablobm pablobm commented Dec 22, 2023

We are not documenting how to add custom assets to complement or override Administrate's own. In fact I recently came across a client project where this was done the wrong way, causing breakage.

So here's some documentation on it, as well as adds examples to the example app. The examples (particularly the CSS one) are a bit artificial, but I couldn't think of anything else. Suggestions welcome.

@pablobm pablobm force-pushed the document-custom-assets branch 7 times, most recently from 8440e60 to 829da14 Compare December 28, 2023 14:37
@pablobm pablobm added the documentation how to use administrate, examples and common usage label Dec 28, 2023
@pablobm pablobm changed the title [WIP] Document custom assets Document custom assets Dec 28, 2023
@@ -0,0 +1 @@
//= require_tree ./admin
Copy link
Contributor

Choose a reason for hiding this comment

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

Hound's got a point. Should this be

/*
*= require_tree ./admin
*/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. To be honest, as long as it works and Hound shuts up, I'm in 👍

@pablobm pablobm marked this pull request as ready for review December 28, 2023 15:05
Comment on lines +98 to +118
## Adding custom CSS and JS

You can add custom CSS and JS to Administrate. Put the files in the
appropriate folders (typically under `assets`) and point Administrate to them
using the following API, preferably in an initializer. For example, if your
files are called `admin.css` and `admin.js`:

```
/// config/initializers/administrate.rb
Administrate::Engine.add_stylesheet("admin")
Administrate::Engine.add_javascript("admin")
```

Then make sure to list them in your manifest file (Rails will helpfully remind
you of this step if you miss it):

```
/// app/assets/config/manifest.js
//= link admin.css
//= link admin.js
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good, thank you!

@pablobm @nickcharlton I wonder what we should to do with the following generators:

  • lib/generators/administrate/assets/assets_generator.rb
  • lib/generators/administrate/assets/javascripts_generator.rb
  • lib/generators/administrate/assets/stylesheets_generator.rb

They are another way of customizing assets. As far as I understand, by running those generators, Administrate's non-compiled assets are copied to the users' app. So users can edit a non-compiled asset and compile it themselves if their app is set up for that. And then their assets will take precedence over the ones from the Administrate gem.

I lean toward removing those generators and making users customize assets by appending to Administrate's CSS/JS, just like the example in this PR, instead of completely replacing them with their own.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. I think that this way of customising assets is more likely to cause issues than to solve them. Adding additional, separate assets to add new behaviour (or override CSS) feels like a better solution to me. Then if anyone wants to fully override Administrate's own assets, they can copy them over manually and do that.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that makes sense to me 👍

@@ -0,0 +1 @@
//= require_tree ./admin
Copy link
Contributor

Choose a reason for hiding this comment

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

Hound's got a point. Should this be

/*
*= require_tree ./admin
*/

Copy link
Member

@nickcharlton nickcharlton left a comment

Choose a reason for hiding this comment

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

I think it'd be good to remove those generators, but in another PR (I'll get around to it eventually, but if someone else wanted to, go ahead!). And then we can merge this once #2397 goes in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation how to use administrate, examples and common usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants