Fixes #1986 - Menu should register child regardless of AA resource load order #1988
Conversation
| # | ||
| if children.has_key? menu_item.id | ||
| menu_item.send :children=, self[menu_item.id].send(:children) | ||
| end |
There was a problem hiding this comment.
That comment section is all well and good, but it doesn't describe what this does.
|
Our Travis builds on master are now green (as of 166bc46). Please rebase your PR on the current master: # Expectations: "upstream" is gregbell's GitHub repo and "origin" is your fork
# Rebase your fork's master branch with the latest upstream changes
git checkout master
git pull --rebase upstream master
git push origin master
# Rebase your feature branch with the latest upstream changes
git checkout your_feature_branch
git pull --rebase upstream master
git push origin your_feature_branch # note that you may need to use -f |
|
@daxter - rebased. |
| it "should return a proc if label was a proc" do | ||
| MenuItem.new(:label => proc{ "Dynamic" }).id.should be_an_instance_of(Proc) | ||
| it "should render a proc if label was a proc" do | ||
| MenuItem.new(:label => proc{ "Dynamic Id" }).id.should == "dynamic_id" | ||
| end |
There was a problem hiding this comment.
Could you add in the opposite of this test?
it "should preserve the ID that was set if the label was a proc" do
MenuItem.new(:label => proc{ "Dynamic Id" }, :id => "static_id").id.should == "static_id"
endAlso, there should be a test that has a proc that will return different values on each call like rand, where we expect that if we call it twice then the ID should be different.
|
@macfanatic can you update this PR? |
…resource load order * ActiveAdmin::MenuItem#dom_id removed, wasn't needed * Load order is no longer important in registering menu items
|
@daxter - rebased off of master with your recommendations in place. |
| end | ||
| raw_id.downcase.gsub( " ", '_' ).gsub( /[^a-z0-9_]/, '' ) |
There was a problem hiding this comment.
IMO this is more readable:
def normalize_id(id)
(id.is_a?(Proc) ? id.call : id).to_s.downcase.gsub ' ', '_'
endNote that I removed both the unconditional rescue and the regex removing non alphanumeric characters. Why are we doing either of those, and why here?
|
Closing this in favor of #2031 |
Fixes #1986.