Skip to content

Update js/small-menu.js #162

Closed
wants to merge 1 commit into from

4 participants

@BFTrick
BFTrick commented Feb 26, 2013

Rather than using the number 600 in multiple places it makes more sense to create a variable that holds the value. It is easier to read and it is easier to update.

@BFTrick BFTrick Update js/small-menu.js
Rather than using the number `600` in multiple places it makes more sense to create a variable that holds the value. It is easier to read and it is easier to update.
c1826b1
@iamtakashi
Automattic member

Nice but one nitpick:) I think it would be cleaner to define the smallScreenWidth inside the existing statement below with a comma.

@sixhours

This is a moot point if we convert to the navigation.js script from Twenty Twelve, see #66

@BFTrick
BFTrick commented Feb 27, 2013

@iamtakashi You're right. If you don't convert to the Twenty Twelve script let me know and I'll update the patch. :)

@iamtakashi
Automattic member

Oh, thanks for the reference, @sixhours. I wasn't aware of that. Mmmm...

@BFTrick
BFTrick commented Feb 27, 2013

@sixhours When are you guys going to decide on the navigation script? This could be a patch in the interim. I'm not sure how many downloads you guys have a day but it may be worth it.

@philiparthurmoore

Probably safe to hold off on this patch request until #66 is dealt with. I don't think we should merge this in the interim.

@sixhours

Just merged #66

@sixhours sixhours closed this Feb 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.