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

Customizing the CSS #103

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Customizing the CSS #103

wants to merge 2 commits into from

Conversation

jwigal
Copy link

@jwigal jwigal commented Dec 10, 2020

Provide an easy way to add your own CSS file on top of the delivered CSS.

@grape-bot
Copy link

2 Warnings
⚠️ There’re library changes, but not tests. That’s OK as long as you’re refactoring existing code.
⚠️ Unless you’re refactoring existing code, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#103](https://github.com/ruby-grape/grape-swagger-rails/pull/103): Customizing the css - [@jwigal](https://github.com/jwigal).

Generated by 🚫 danger

@dblock
Copy link
Member

dblock commented Dec 10, 2020

I have some questions.

Why would you rather do this than include the additional stylesheet in your app/assets/stylesheets/application.css?

Assuming we still want this feature, I think we should make it more generic and allow overriding all CSS, i.e. default it to the built-in CSS. I propose that we name this stylesheet_paths and make it an Array.

GrapeSwaggerRails.options.stylesheet_paths << 'some.css' # [ 'grape_swagger_rails/application.css', 'some.css' ]

Then

  <% GrapeSwaggerRails.options.stylesheet_paths.each do |stylesheet_path| %>
    <%= stylesheet_link_tag stylesheet_path %>
  <% end %> 

To be merged this needs tests, too, please.

If you really want to be ambitious, users should also be able to configure script_paths, i.e. all the JS.

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.

3 participants