loaf
loaf copied to clipboard
Add :http_verbs option
Describe the change
Add :http_verbs
option to breadcrumb parameters
Why are we doing this?
When I use this gem on some legacy projects, I met a circumstance which I have to show breadcrumbs on POST/PATCH action. I tried to add an option :http_only
at #49. But @piotrmurach suggest me to use http_verbs
.
Benefits
Now the current breadcrumb can match with multiple HTTP methods
breadcrumb 'Step 1', onboard_step_path(step: 1), http_verbs: [:get]
breadcrumb 'Step 2', onboard_step_path(step: 2), http_verbs: [:get, :post, :delete]
breadcrumb 'Step 3', onboard_step_path(step: 3), http_verbs: :all
Thank you Hopper for making these changes! Things are looking really good. There is a small style issue left.
There is one dilemma that I'm 'debating' which is the name of the parameter
:http_verbs
. Theverbs
term is kind of inaccurate because these are not all verbs and also it is rather 'colloquial' usage. The HTTP rfc talks about HTTP methods. It feels more appropriate to usemethods
. I'm equally debating whether it should berequest_methods
orhttp_methods
? What're your thoughts on this? I want to make sure we pick the best configuration name.
Maybe the request_methods
is better, as Rack is using request_method
.
https://github.com/rack/rack/blob/d15dd728440710cfc35ed155d66a98dc2c07ae42/lib/rack/request.rb#L174-L202
Apologies for not having replied to your request for review earlier. I took a break from open-source. I left a few comments. Let's move this PR forward.
Glad to hear that! I'll get it done sometime this week.
I just fixed all those style issues. Also fixed a wrong proc config test issue
Are there plans to merge these changes?