[Obs AI Assistant] Simplify routing #176569
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
| replace: (path, ...args) => { | ||
| const next = link(path, ...args); | ||
| history.replace(next); | ||
| }, | ||
| navigateToConversationsApp: (path, ...args) => { |
There was a problem hiding this comment.
curious why we need this in any case?
There was a problem hiding this comment.
I don't think we necessarily do. I'd prefer only having push/replace but that would require changing them to use navigateToApp (see conversation here).
There was a problem hiding this comment.
Removing the navigateToConversationsApp method in 20c6148
sorenlouv
added
release_note:skip
| }, | ||
| replace: (path, ...args) => { | ||
| const next = link(path, ...args); | ||
| history.replace(next); | ||
| navigateToApp('observabilityAIAssistant', { path: next, replace: true }); |
There was a problem hiding this comment.
nice, does this not cause a full page refresh?
There was a problem hiding this comment.
No. I was also a little worried about using this internally when the name suggests it should be used when navigating between apps. But I don't observe any downsides like full page refreshes.
There was a problem hiding this comment.
Replace does mean you can't go use browser back anymore no?
There was a problem hiding this comment.
Replace does mean you can't go use browser back anymore no?
Yes, instead of adding a new item to the history stack (like push) replace overwrites the existing item.
sorenlouv
force-pushed
the
navigate-to-app
branch
from
20c6148 to
64274cc
Compare
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
Minor simplification to how we link into the conversation app
Minor simplification to how we link into the conversation app
Minor simplification to how we link into the conversation app