-
Notifications
You must be signed in to change notification settings - Fork 374
fix: correct URL when switching page modes #4736
Conversation
| const { dialogId = '', projectId = '', skillId = null } = props; | ||
| const dialogs = useRecoilValue(validateDialogsSelectorFamily(skillId ?? projectId)); | ||
|
|
||
| console.log(skillId, projectId, skillId ?? projectId, dialogs); |
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.
remove logs
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.
Oops. Thanks for catching this.
| const { dialogId = '', projectId = '', skillId = null } = props; | ||
|
|
||
| const baseURL = | ||
| projectId === skillId || skillId == null ? `/bot/${projectId}/` : `/bot/${projectId}/skill/${skillId}/`; |
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.
projectId === skillId should never happen correct?
| const edit = /\/edit(\/)?$/.test(path); | ||
|
|
||
| const baseURL = | ||
| projectId === skillId || skillId == null ? `/bot/${projectId}/` : `/bot/${projectId}/skill/${skillId}/`; |
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.
same issue here projectId===skillId is a scenario that should never occur
| }>> = (props) => { | ||
| const { dialogId = '', projectId = '' } = props; | ||
| const dialogs = useRecoilValue(validateDialogsSelectorFamily(projectId)); | ||
| const { dialogId = '', projectId = '', skillId = null } = props; |
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.
is there a reason why skillId is instanitated to null and other props are'nt? Can we make the instantiation consistent?
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 originally had this as '', but that broke the ?? melding I was doing. If anything, I think they should all default to null, so I'll make that change too.
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.
Update on this - changing them all to null caused more typechecking issues downstream, so I found another solution that looks less strange than this. We need a projectId and dialogId in all cases, but the skillId is optional and gets marked as such.
hatpick
left a comment
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 use:
`${baseUrl}restOfUrl`
instead of + concat.
* settingsPage: remove useless files Corrected types Support undo/redo for skill (microsoft#4754) fix UT feat: Diagnostics Page (microsoft#4457) set defaultLanguage as active language if active language is deleted fix: correct URL when switching page modes (microsoft#4736) fix: do not stub __dirname for node extensions (microsoft#4749) chore: bundle extensions (microsoft#4745) fix: Delete & Undo on actions with LG templates doesn't bring back the LG content (microsoft#4740) feat: update sdk package to 4.11.0 (microsoft#4741)
Description
This adds awareness of the project and skill structure to the LG, LU, and QnA pages. They will now display the dialogs of whichever the current selected bot or skill is, keeping that around in the URL and using it to correctly point to the right place back in Design View.
Task Item
closes #4615