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 a generator for the sidebar #328

Closed

Conversation

acrogenesis
Copy link
Contributor

I needed to edit the sidebar and noticed there wasn't a generator so I created one.
Run

rails g administrate:views:sidebar

To generate it, already verified on my app it works and tests are ✅

expected_contents = contents_for_application_template("_sidebar")

run_generator []
contents = File.read(file("app/views/admin/application/_sidebar.html.erb"))

Choose a reason for hiding this comment

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

Line is too long. [81/80]

@c-lliope
Copy link
Contributor

acrogenesis, thanks!

I merged in #269 yesterday, which has a lot of overlap with this. I think there's room for both of:

rails generate administrate:views:layout
rails generate administrate:views:sidebar

I think we'll want to change the layout generator so it calls this generator, instead of duplicating the logic.

If you want to look into this, great! Otherwise I can take care of this when I merge it.

@acrogenesis
Copy link
Contributor Author

I'm not entirely sure how to invoke the generator from the LayoutGenerator I'm trying to do it calling Rails::Generators.invoke("administrate:views:sidebar") but the tests won't pass

@c-lliope
Copy link
Contributor

Yep, that's the right way to do it. The tests won't execute the sub-generator, so you'll have to stub it like I did here:

%w[images javascripts stylesheets].each do |asset|
expect(Rails::Generators).
to invoke_generator("administrate:assets:#{asset}")

(note the invoke_generator method defined below.

contents = File.read(generated_file)

expect(contents).to eq(expected_contents)
expect(Rails::Generators).to invoke_generator("administrate:views:sidebar")

Choose a reason for hiding this comment

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

Line is too long. [81/80]

@acrogenesis
Copy link
Contributor Author

thanks! I've updated the generator and tests. I also added the invoke_generator method to the generator helpers.

@c-lliope
Copy link
Contributor

Great, thanks! I'll review this in more detail in the morning, and get it merged in as soon as I can!

@@ -15,6 +15,10 @@ def contents_for_application_template(view_name)
)
end

def invoke_generator(*args)
have_received(:invoke).with(*args)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice move.

@c-lliope
Copy link
Contributor

This looks good! I'll shuffle the lines around so they're < 80 characters, and merge this in.

@c-lliope
Copy link
Contributor

Merged in as 04ee51c. Thanks!

@c-lliope c-lliope closed this Dec 15, 2015
@acrogenesis acrogenesis deleted the sidebar-generator branch December 15, 2015 15:29
c-lliope added a commit that referenced this pull request Jan 22, 2016
Changes:

```
* [#269] [FEATURE] Add a generator for copying default layout files
* [#328] [FEATURE] Add a generator for copying default sidebar partial
* [#362] [FEATURE] Add a generator for only the dashboard manifest.
  Customizing this manifest before running the `administrate:install` generator
  will change which dashboards get generated.
* [#295] [FEATURE] Add dashboard detection for ActiveRecord::Enum fields.
* [#364] [FEATURE] Improve dashboard generator by explicitly listing out the
  generated `SHOW_PAGE_ATTRIBUTES` array elements.
* [#416] [UI] Add an accessibility label to the search input
* [#411] [UI] Use tabular figures in table cells
* [#409] [UI] Use default system fonts
* [#424] [BUGFIX] Fix a bug where running `rails destroy GENERATOR_NAME`
  would not work for several of the generators
* [#390] [BUGFIX] Fix timestamp deprecation warnings
* [#365] [COMPAT] Remove dependency on `inline_svg`
* [#396] [I18n] Ukrainian
* [#297] [I18n] Italian
* [#307] [I18n] Fix German grammatical errors
* [#363] [DOC] Move documentation into main repository, at the root URL
* [#395] [DOC] Update inline documentation for collection partial
* [#387] [DOC] Fix incorrect path for generators in the docs
```
@c-lliope c-lliope mentioned this pull request Jan 22, 2016
c-lliope added a commit that referenced this pull request Jan 22, 2016
Changes:

```
* [#269] [FEATURE] Add a generator for copying default layout files
* [#328] [FEATURE] Add a generator for copying default sidebar partial
* [#362] [FEATURE] Add a generator for only the dashboard manifest.
  Customizing this manifest before running the `administrate:install` generator
  will change which dashboards get generated.
* [#295] [FEATURE] Add dashboard detection for ActiveRecord::Enum fields.
* [#364] [FEATURE] Improve dashboard generator by explicitly listing out the
  generated `SHOW_PAGE_ATTRIBUTES` array elements.
* [#416] [UI] Add an accessibility label to the search input
* [#411] [UI] Use tabular figures in table cells
* [#409] [UI] Use default system fonts
* [#424] [BUGFIX] Fix a bug where running `rails destroy GENERATOR_NAME`
  would not work for several of the generators
* [#390] [BUGFIX] Fix timestamp deprecation warnings
* [#365] [COMPAT] Remove dependency on `inline_svg`
* [#396] [I18n] Ukrainian
* [#297] [I18n] Italian
* [#307] [I18n] Fix German grammatical errors
* [#363] [DOC] Move documentation into main repository, at the root URL
* [#395] [DOC] Update inline documentation for collection partial
* [#387] [DOC] Fix incorrect path for generators in the docs
```
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