Fixing the menu highliting #19

Merged
merged 2 commits into from Mar 30, 2013

Conversation

Projects
None yet
4 participants
Owner

PragTob commented Mar 30, 2013

Hi all,

the first commit removes the static active class from home.
The second commit adds highliting based on the current page back in again. I dunno how to properly make helpers/partials with HAML/Webmachine so there's a bit of duplication but I think it suites its purpose.

If anyone knows how to handle this current menu item highliting better, I'd be intrigued to know about it :-D

Shoes on!
Tobi

PragTob added some commits Mar 30, 2013

jrgifford added a commit that referenced this pull request Mar 30, 2013

Merge pull request #19 from PragTob/LinkFixing
Fixing the menu highliting

@jrgifford jrgifford merged commit 93ca92d into shoes:master Mar 30, 2013

@@ -6,7 +6,8 @@ def initialize(view)
end
def render(locals={})
- Haml::Engine.new(File.read("views/layout.html.haml")).render do
+ locals.merge! page: @view
@steveklabnik

steveklabnik Mar 30, 2013

Owner

In theory, this should really be something like

locals = locals.dup.merge! page: @view

So that you don't mutate the hash that was passed in. That said, I don't think it really matters here.

@PragTob

PragTob Mar 30, 2013

Owner

Thanks for the Feedback Steve!
What would the general concern with mutating the Hash here be? (just curious, only answer if you got time).

Plus: couldn't I just do locals = locals.merge page: @view, or is there something very different about #dup ?

@steveklabnik

steveklabnik Mar 30, 2013

Owner

What would be the general concern

It's not polite to mutate things in Ruby unless it's expected. That's why there's merge and merge! in the first place, no?

Plus: couldn't I just do

yes, that'd work too.

@PragTob

PragTob Mar 30, 2013

Owner

Yep gotta work on immutability and specifically my functional programming. Thanks for elaborating!

Contributor

wpp commented Mar 30, 2013

@PragTob Thank you for fixing this!

Owner

PragTob commented Mar 30, 2013

@wpp nonono thank you for making this website totally awesome =)

Contributor

wpp commented Mar 30, 2013

😳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment