-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[RFC]: Add support for fragments/anchors to routes
functions
#8069
Comments
routes
functionroutes
functions
Happy to look into fixing this |
It's all yours! :) |
Updated the post with more rationale about going with just a key named |
TIL! I thought you still did routes.post({ id: 123, fragment: 'comments' }) I'm not a big fan of mixing special meaning keys with user defined keys in the same namespace. Doing that has already come back to bite us so many times! A recent example is here: #7024 but it's also been when we inject props into pages etc If you're still not convinced it's a bad idea, maybe we could consider naming the key routes.post({ id: 123, '#': 'comments' }) FWIW I don't think this is too bad... <Link to={routes.post({ id: 123 }) + '#comments'}>See Comments</Link> Why introduce new syntax for something that's already possible to do? |
Hmm, I do find that
That's a slippery slope argument where you end up talking yourself out of every feature we've created for the framework! "It's already all possible, why use Redwood?" For me, I think if the |
Great! So remind me again how you use the |
WELL, the routes are only for internal routing, right? So no need to set the scheme (or domain, or port). But hey I'm up for adding it if you are. ;) |
The |
I have seen a few PRs related to this route subject. Is this now done or do I still need to look into this ticket? |
A PR for this would still be very much appreciated 🙂 |
routes
functionsroutes
functions
Summary
A "fragment" is the HTML spec's name for the part of the URL after a
#
symbol:The default browser behavior is to scroll down to the first element in the DOM that has an
id
attribute matching that fragment:Right now there's no way, using the standard
routes
helper functions, to construct a URL that includes a fragment. A workaround is to append it to the generated path:Motivation
Fragments are a standard part of URLs and part of the HTML spec, and we should be able to create those URLs using our recommended tools (the
routes
helpers). It's very common to want to redirect somewhere else and jump to a certain section of the page.Note that depending on how the page is build, the required DOM element may not be present on the page, and so the browser will not actually be able to scroll down to it. I would like to support this behavior as well, but this Issue is specifically just about being able to create the URL.
Detailed proposal
I propose adding support for a
fragment
key to the namedroutes
helpers, in some form or another. One possibility is:This means that you could no longer create a query string variable named
fragment
, which would make this change backwards incompatible for anyone that was already using that name. (Today, if you wrote the above code, you'd get the path/post/123?fragment=comments
I'd also be open to prefixing this key with something, such as an underscore like
_fragment
, which indicates that it has special significance as far as Redwood is concerned. My only concern is that informally in the JS community, prefixing something with an underscore denotes it as "private" functionality that the end user generally shouldn't mess with.Another option would be to have these helpers accept a second, optional argument:
This would be completely new functionality and thus not break existing route generation.
For what it's worth, in the Rails helpers this key is named
anchor
and that's just the way it is: you can't useanchor
for a query string variable and it's fine.It's possible we could use
fragment
but provide a codemod that searches through your codebase for any usage ofroutes.something({ fragment: '' })
and give you a warning? It would be nice to just pick something and go for it, rather than wring our hands that maybe someone used that word, and make the UX worse for everyone else for all time.Are you interested in working on this?
The text was updated successfully, but these errors were encountered: