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

Add current_page? helper method to pages and components #1073

Closed
paulcsmith opened this issue Mar 30, 2020 · 5 comments
Closed

Add current_page? helper method to pages and components #1073

paulcsmith opened this issue Mar 30, 2020 · 5 comments
Labels
feature request A new requested feature / option

Comments

@paulcsmith
Copy link
Member

This will be used by #969

Should work similarly to Rails https://apidock.com/rails/ActionView/Helpers/UrlHelper/current_page%3F

current_page?(User::Index)
current_page?(Users::Show.with(user.id))
current_page?("/raw/string")

By default it should not care about query params but passing check_query_params: true will include the path and query params

cc @wout

@paulcsmith paulcsmith added the feature request A new requested feature / option label Mar 30, 2020
@wout
Copy link
Contributor

wout commented Mar 31, 2020

@paulcsmith Do you want it to work for full uri's, including scheme, host and port? Or only for path or path + query?

@paulcsmith
Copy link
Member Author

@wout Great question. I think host, scheme and port should be options. But I'd also be ok shipping those in a separate PR to get the basic use case of path done. Up to you though!

@wout
Copy link
Contributor

wout commented Apr 1, 2020

Ok, I'll have a look at host and port as well. However, I need a pointer here since Crystal's HTTP::Request does not carry scheme information. Is it available somewhere in Lucky?

I was also wondering if it is relevant these days. I don't know all the use cases, but isn't it either http or https? And if so, since https is becoming a requirement these days, should we even test on scheme?

@paulcsmith
Copy link
Member Author

@wout Great point. We can skip that I think. Everyone should be using ssl and Lucky makes it easy to force with the ForceSslHandler. Also scheme shouldn't change what page is shown as long as the rest of the host and path match 👍

@wout
Copy link
Contributor

wout commented Apr 1, 2020

@paulcsmith That's done then. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A new requested feature / option
Projects
None yet
Development

No branches or pull requests

2 participants