Custom app paths #846
Custom app paths #846
Conversation
- Add Jest - parsePath(): Move history.replace() outside of parsePath(), to remove any side effect from the function. - Send the current custom path to the connected app. - Update the browser path based on the path requested by the connected app.
src/routing.test.js
Outdated
| }) | ||
| ) | ||
|
|
||
| path = `/p/${ADDRESS}///` |
There was a problem hiding this comment.
I don’t think anyone will use a slash followed by another one, but I don’t see a reason to forbid it. Do you think it should be the case?
| @@ -107,26 +104,27 @@ export function getAppPath({ | |||
| return `/${action}` | |||
| } | |||
|
|
|||
| if (!fullDao) { | |||
| return `/${search}` | |||
| // Only keep the DAO name if it ends in aragonid.eth | |||
There was a problem hiding this comment.
It seems a bit odd that we're doing this here in routing, but moving the redirect out into the app itself.
There was a problem hiding this comment.
The redirect used to be done here, but I don’t think this function should have any side effect. I don’t think it should stay in App either, I was thinking of moving all of the history / window.location handling in the routing provider that remain to be added. What do you think?
There was a problem hiding this comment.
I was thinking of moving all of the history / window.location handling in the routing provider that remain to be added
Yes! This sounds like exactly what we need!
Co-Authored-By: Brett Sun <qisheng.brett.sun@gmail.com>
| // params need to be a string | ||
| handleParamsRequest = params => { | ||
| this.openApp(this.props.locator.instanceId, { params }) | ||
| handleAppMessage = ({ data: { name, value } }) => { |
There was a problem hiding this comment.
😅 I didn't even realize before that we removed this function and we were always passing down undefined
| @@ -107,26 +104,27 @@ export function getAppPath({ | |||
| return `/${action}` | |||
| } | |||
|
|
|||
| if (!fullDao) { | |||
| return `/${search}` | |||
| // Only keep the DAO name if it ends in aragonid.eth | |||
There was a problem hiding this comment.
I was thinking of moving all of the history / window.location handling in the routing provider that remain to be added
Yes! This sounds like exactly what we need!
|
@bpierre I've pushed an update to fix the hook dependencies :). |
history.replace()outside ofparsePath(), to remove any side effect.?p=) support.