Portal toasters into app provider + Fix toaster hrefs to include path prefix. #16696
Conversation
|
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit 68a197b. |
salazarm
changed the title
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 68a197b. |
js_modules/dagster-ui/packages/ui-components/src/components/createToaster.tsx
Show resolved
Hide resolved
| @@ -71,6 +71,15 @@ const setup = (instance: ToasterInstance): DToaster => { | |||
| </> | |||
| ); | |||
| } | |||
| if (rest?.action?.href) { | |||
There was a problem hiding this comment.
This is an interesting fix but I am worried this behavior is inconsistent with the rest of the UI kit -- if you make an <a href= or a window.open(href) call, you have to pass the correct href. Why should toaster let you pass an incorrect href and fix it for you?
We have some places like RunUtils.tsx where we already pass the href correctly built from history.createHref. Can you update the callsites that are doing it incorrectly to do it right instead?
I'm worried that these places that ARE doing it right will have the href "double prepended" with the path prefix now.
There was a problem hiding this comment.
Alternatively, we could extend the Toaster data to let you specify action: {text: string, to: string} and then have the toaster transform the "to" into an href. I think that'd be consistent with how <Link works and would be nice!
There was a problem hiding this comment.
We have some places like RunUtils.tsx where we already pass the href correctly built from history.createHref. Can you update the callsites that are doing it incorrectly to do it right instead?
Yeah I was considering doing that too but the utilities creating the href aren't hooks so they can't access createHref. I guess I can update their callsites to pass the history object from useHistory to them. We could also check if it already has the prefix and if so skip adding it.
Summary & Motivation
Fixes #16690
How I Tested These Changes
Locally used --path-prefix and made sure the toast had the correct href