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

Fix highlighting of "Consultations" in main navigation #4

Merged
merged 1 commit into from Feb 28, 2012
Merged

Fix highlighting of "Consultations" in main navigation #4

merged 1 commit into from Feb 28, 2012

Conversation

tomstuart
Copy link
Contributor

Broken by 4b5f570.

This is a band-aid over the underlying problem that there are no tests keeping current_main_navigation_path in sync with the contents of the #global-nav element in website.html.erb.

I haven't fixed that here since it requires a decision about whether to simply write such tests against the current code, or rewrite that code to remove duplication between #global-nav and the helper (e.g. use params instead of named route helpers in the main_navigation_link_to calls in #global-nav), or both.

Broken by 4b5f570.

This is a band-aid over the underlying problem that there are
no tests keeping current_main_navigation_path in sync with the
contents of the #global-nav element in website.html.erb.

I haven't fixed that here since it requires a decision about
whether to simply write such tests against the current code,
or rewrite that code to remove duplication between #global-nav
and the helper (e.g. use params instead of named route helpers
in the main_navigation_link_to calls in #global-nav), or both.
lazyatom added a commit that referenced this pull request Feb 28, 2012
Fix highlighting of "Consultations" in main navigation
@lazyatom lazyatom merged commit b3f88b7 into alphagov:master Feb 28, 2012
@lazyatom
Copy link
Contributor

Thanks for this Tom!

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.

None yet

2 participants