Refactored _menu and _menu_branch partials. #2068
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Should |
| } -%> | ||
| </ul> | ||
| </nav> | ||
| <%= content_tag(:nav, :id => dom_id, :class => css) do %> |
There was a problem hiding this comment.
This seems weird to me. content_tag has always seemed like a one-off to use in helpers or something. I'm not sure I'd find this more readable than just
<nav id=#{dom_id} class=#{css}>
# ...
</nav>
There was a problem hiding this comment.
Heh, I just saw that you deleted this in the other PR. Nevermind...
* Using content_tag helpers. * Passing way less data around. * Removed :apply_css option, just pass :css => '' * Removed logic from views by adding to view helpers. * Removed Refinery::Core.hide_menu_children configuration option. * Added MenuItem#ophan? which returns !MenuItem#has_parent? * Removed :hide_children menu option in favour of just using :menu_levels => 0. In effect, they did the same thing.
This has many advantages not least of which is that we can test it without requesting pages. * Using config_accessor and attr_accessor instead of passing options around. * Added leaf? to MenuItem and made use of has_children? in the MenuPresenter * All methods besides initialize and to_html are private to this class. * Moved Refinery::Core#menu_css to the Refinery::Pages::MenuPresenter. * Uses MenuItem#depth instead of scanning ancestors. * Renamed levels to max_depth and added within_max_depth? * Refactored Menu and MenuItem as proper objects, not pretending to be hashes or arrays (ht: @awagener) * Move Menu#initialize logic to Menu#append so that more items can be appended after initialize. * Moved all page presenters and their specs under app/presenters and spec/presenters where they belong.
Replaced menu partials with Refinery::Pages::MenuPresenter.
robyurkowski
added a commit
that referenced
this pull request
Dec 18, 2012
Refactored _menu and _menu_branch partials.
|
@parndt Friendly reminder to changelog this! |
ugisozols
added a commit
that referenced
this pull request
Dec 18, 2012
|
Good point; will do! |
|
I already did ;) |
|
@ugisozols thanks that didn't show up in my email. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
content_taghelpers.:apply_cssoption, just pass:css => ''to apply no CSS.MenuItem#orphan?which returns!MenuItem#has_parent?:hide_childrenmenu option because:menu_levelsdoes the same thing.