Skip to content

Conversation

@amatsuda
Copy link
Collaborator

This patch refactors the module that defines nav_item_class and subnav_item_class from a spaghetti jungle that pollutes all controllers with 13 methods to a super elegantly written fine tuned short piece of readable code that does not pollute anything at all.

to be clearer about what it returns.
because it returns a Regexp for matching path prefixes.
feels like it's nothing but this.
Currently, if no nav matches, both `@active_nav_key` and `@active_subnav_key`
stay nil, and nil || nil is falsy, so we recalculate on every call.
to reduce methods and ivars
so the given request.path can dynamically match using current_event.
This eliminates creation of Regexp objects, and reduces execution of
url_for helpers.
since all path helpers are dynamically executed now, we can share this
whole navigation map globally across all requests now.
so we can eliminate the Array case
where it used to be done twice. Also, we could finally eliminate a
method that was defined for all controllers with too general name
"match?".
with an assumption that subnav_item_class is always called AFTER
nav_item_class was called at least once.
No need to define them in controllers, because we call them only from
view files.
because we don't use this anywhere else.
@amatsuda amatsuda merged commit 48c80b4 into rubycentral:main Dec 26, 2025
1 of 2 checks passed
@amatsuda amatsuda deleted the retroactive_navigation branch December 26, 2025 16:06
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.

1 participant