Skip to content
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

fix: generate correct URLs for root bots #4614

Merged
merged 3 commits into from
Nov 2, 2020
Merged

Conversation

beyackle
Copy link
Contributor

@beyackle beyackle commented Nov 2, 2020

Description

This fixes a small glitch in generating URLs for the links; dialogs and triggers that belong to the root of a botProject now generate links that look like the older non-botProject ones, which should fix some issues with undo/redo.

Task Item

#minor

Screenshots

image

@beyackle beyackle changed the title Update ProjectTree.tsx fix: generate correct URLs for root bots Nov 2, 2020
@@ -279,7 +279,7 @@ export const ProjectTree: React.FC<Props> = ({
displayName: dialog.displayName,
isRoot: dialog.isRoot,
projectId: rootProjectId,
skillId: skillId,
skillId: skillId === rootProjectId ? undefined : skillId,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@beyackle  I see a bug where I click on a skill's dialog file url changes correctly to http://localhost:3000/bot/25141.842060596977/skill/5927.301138798824/dialogs/googlekeepsync?selected=triggers[%22376720%22] . Navigate to a different page and come back to the DesignPage. The url is an invalid URL. It needs to set the url obtained from the previous designPageLocationState right?

Copy link
Contributor Author

@beyackle beyackle Nov 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks to me like a problem in the sidebar - when I have a skill open (so, the URL is /bot/<bot Id>/skill/<skill Id>) and click on the sidebar button, it takes me to a URL with /bot/<skill Id>/ in it, and then clicking back into Design keeps that around. Single-bot projects don't have this problem. The reason this change is going into main and not the feature branch is that undo/redo was breaking even in the single-bot case, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix has no relation to undo/redo. The URL was broken irrespective of whether we were doing Undo/redo.

Copy link
Contributor

@srinaath srinaath Nov 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be an issue with the sidebar though. DesignPageLocation state stores the current focussed actions etc that are particular to the design page. On rendering the designPage it should always use the currentProjectId state and parse the designPagelocation state to set the URL correctly?

@coveralls
Copy link

coveralls commented Nov 2, 2020

Coverage Status

Coverage increased (+0.008%) to 55.148% when pulling 5f77a6d on beyackle/fixNonSkillURLs into 2031750 on main.

@beyackle beyackle merged commit 77ad8e1 into main Nov 2, 2020
@beyackle beyackle deleted the beyackle/fixNonSkillURLs branch November 2, 2020 20:31
luhan2017 pushed a commit that referenced this pull request Nov 3, 2020
@cwhitten cwhitten mentioned this pull request Nov 13, 2020
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants