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 section class to <html>; Clear up helper use #31

Merged
merged 2 commits into from
Sep 29, 2017
Merged

Conversation

techgique
Copy link
Member

Add class="site_(section)" to element
Application helper site_section called from application layout view
Uses the first value from:

  • Manual override in @site_section instance variable
  • "home" if a request for the home page
  • A sub-uri or namespace name in the path
  • Path name if it matches the controller or action name
  • "pages" as the last default

Change generator to copy extendable helper files to main app

Remove Orchid::ApplicationHelper include from Orchid::ItemsHelper as
included app-wide by application_helper.rb copied to main app

Close #15

Add class="site_(section)" to <html> element
Application helper site_section called from application layout view
Uses the first value from:
- Manual override in @site_section instance variable
- "home" if a request for the home page
- A sub-uri or namespace name in the path
- Path name if it matches the controller or action name
- "pages" as the last default

Change generator to copy extendable helper files to main app

Remove Orchid::ApplicationHelper include from Orchid::ItemsHelper as
included app-wide by application_helper.rb copied to main app
# Use override from instance variable
section = @site_section
else
if current_page?(home_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

since this is a rails method, would it fit the style guide better without parentheses? I prefer to read it this way, honestly, but I wanted to see if I have the wrong idea in mind for the style guide's preferences.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right. I think I wrote it that way because I looked at the API and all their examples are written with parentheses. http://api.rubyonrails.org/classes/ActionView/Helpers/UrlHelper.html#method-i-current_page-3F

I'll fix it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

If you decide you would be okay with switching everything ever to parentheses, let me know, and I will start search and replacing immediately with gusto!

@@ -121,6 +122,13 @@ def gitignore
return "Orchid .gitignore file copied to app's root directory"
end

def helpers
FileUtils.rm("#{@new_app}/app/helpers/application_helper.rb")
FileUtils.cp(Dir.glob("#{@this_app}/app/helpers/*_helper.rb"), "#{@new_app}/app/helpers/")
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose I could test this out, but this is only copying the reference helpers, and not searching recursively the copy the actual filled out helpers, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. As intended. If we copy the actual helpers, then Orchid updates would need to be copied into the main app.

This uses the fancier path searching where it knows to look in engines' helper directories for the include files. Similar to how the include directive in the main app's application.js will look in the asset directories of engines, gems, vendor/, etc and can grab orchid.js from the Gem without copying the file into the main app.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, that's why I wanted to double check that it was only grabbing the referencing helpers. Cool deal. :)

@jduss4 jduss4 merged commit 7a58530 into master Sep 29, 2017
@jduss4 jduss4 deleted the site-classes branch September 29, 2017 00:06
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.

2 participants