-
Notifications
You must be signed in to change notification settings - Fork 66
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
Possibility to add optional parameters #186
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be really cool! Could you add tests to verify that this is working as intended?
I signed it!
2017-03-16 15:08 GMT+01:00 googlebot <[email protected]>:
… Thanks for your pull request. It looks like this may be your first
contribution to a Google open source project. Before we can look at your
pull request, you'll need to sign a Contributor License Agreement (CLA).
📝 *Please visit https://cla.developers.google.com/
<https://cla.developers.google.com/> to sign.*
Once you've signed, please reply here (e.g. I signed it!) and we'll
verify. Thanks.
------------------------------
- If you've already signed a CLA, it's possible we don't have your
GitHub username or you're using a different email address. Check your
existing CLA data <https://cla.developers.google.com/clas> and verify
that your email is set on your git commits
<https://help.github.com/articles/setting-your-email-in-git/>.
- If you signed the CLA as a corporation, please let us know the
company's name.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#186 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AF3lkrmnaRYlytsT37GafEw_1F9J8sAxks5rmUJ3gaJpZM4MfWaJ>
.
--
José Luis Ramos <http://google.com/+JoséLuisRamosRomero>
www.futbolypensamiento.com
|
CLAs look good, thanks! |
Of course, I get on to it now!
2017-03-16 15:14 GMT+01:00 Tim van der Lippe <[email protected]>:
… ***@***.**** requested changes on this pull request.
This would be really cool! Could you add tests to verify that this is
working as intended?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#186 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AF3lkkJoYeK0VjJ6PWPnQz9k5YLB26bgks5rmUOsgaJpZM4MfWaJ>
.
--
José Luis Ramos <http://google.com/+JoséLuisRamosRomero>
www.futbolypensamiento.com
|
Any news about this feature ? |
Optional parameters will be so cool and really help the routing process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you accidently reverted some configuration which broke the Travis CI build
test/index.html
Outdated
'app-location.html', | ||
'test-observer-app.html', | ||
'test-app-example-1.html', | ||
'app-route-optional-params.html' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert the changes to the test configuration to make sure we do not regress.
Revert changes
We have the possibility to add optional parameters on the pattern attribute with the
?
character at the end.In this example:
The path
/client
matches the route, settingrouteData.id
toundefined
andactive
totrue
. If pattern would be/client/:id
, this path wouldn't matches the route. With the pattern of the example above, the path/client/c123
also matches the route, withrouteData.id
equal toc123
.A more complicated example would be the same app-route element with pattern attribute
pattern="/client/:id?/:action?"
. The paths/client
,/client/
,/client/c123
,/client/123/overview
also matches the route. Supossing second path (/client/
),routeData
would be{'id': '', 'action': undefined}
This feature also works with fixed patterns without parameters (i.e.
pattern="/client/list?"
).