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

chore: add onOpenDialog api to support dialog jump #4542

Merged
merged 8 commits into from
Nov 30, 2020

Conversation

yeze322
Copy link
Contributor

@yeze322 yeze322 commented Oct 27, 2020

Description

closes #3942

Previously, we use navTo() to support both breadcrumb jump and BeginDialog jump. It has several problems

  1. Updating navTo api broke the dialog jump behavior, we fixed this bug.
  2. The navTo() API belongs to ApplicationContextApi while Flow Editor prefers to consume DialogEditingContextApi. Using navTo breaks the context boundary.

For providing a more descriptive interface, this PR implements the new api onOpenDialog used only in the dialog jump scenario. In the future, we can freely update the navTo() api without bothering breaking the dialog jump behavior.

Task Item

Screenshots

@yeze322 yeze322 changed the title chore: add onOpenDialog api to support goto dialog chore: add onOpenDialog api to support dialog jump Oct 27, 2020
@boydc2014
Copy link
Contributor

boydc2014 commented Oct 27, 2020

This looks reasonable to me for creating a dedicated api in this BeginDialog jump.

My question is should we just update the logic to use this new API, so that we will know how much changes is needed or what the impact is. That will help us to determine whether take this or not, right?

@yeze322
Copy link
Contributor Author

yeze322 commented Oct 27, 2020

@boydc2014 The new API is consumed in useEditorEventApi.tsx where handles the click event on dialog link. That's the only usage for now, it seems not noticeable.

@boydc2014
Copy link
Contributor

boydc2014 commented Oct 27, 2020

@boydc2014 The new API is consumed in useEditorEventApi.tsx where handles the click event on dialog link. That's the only usage for now, it seems not noticeable.

Oh, get it

boydc2014
boydc2014 previously approved these changes Oct 27, 2020
@a-b-r-o-w-n a-b-r-o-w-n added the 1.3 1.3 Release label Nov 13, 2020
@cwhitten
Copy link
Member

@yeze322 can you address conflicts

@coveralls
Copy link

coveralls commented Nov 18, 2020

Coverage Status

Coverage decreased (-0.002%) to 55.399% when pulling daff3b9 on yeze322:refactor/openDialog into 6ad4945 on microsoft:main.

@cwhitten
Copy link
Member

@yeze322 still have conflicts

@cwhitten cwhitten merged commit e4c6c71 into microsoft:main Nov 30, 2020
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
* impl onOpenDialog as DialogEditingContext

* add missing api in stub

Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Co-authored-by: Dong Lei <donglei@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.3 1.3 Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a new navigation API to handle jumping between dialogs
5 participants