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.

Sign up for GitHub

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

Jump to bottom

fix: Can not add properties in "SetProperties" action #4572

Merged
merged 5 commits into from
Oct 28, 2020
Merged

fix: Can not add properties in "SetProperties" action #4572

merged 5 commits into from
Oct 28, 2020

Conversation

LouisEugeneMSFT
Copy link
Contributor

Description

SetProperties leverages the ArrayFieldItem component. On blur, the item is removed if it has an empty value:
image

The children of SetProperties rely on Intellisense fields. But the onBlur callback was not passed in the array of dependency of the useEffect responsible for managing Intellisense blur. Therefore the initial (empty) value was used and always removed the item, regardless of the current real value. Adding to the array of dependency fixed the issue.

This PR also contains some minor optimizations (rendering and listeners) for Intellisense.

closes #4526

Screenshots

image

@lgtm-com
Copy link

lgtm-com bot commented Oct 28, 2020

This pull request introduces 1 alert when merging 8563fa9 into 5bf5199 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@coveralls
Copy link

coveralls commented Oct 28, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.003%) to 55.169% when pulling 90408e8 on leugene/issue4526 into 5bf5199 on main.

GeoffCoxMSFT
GeoffCoxMSFT previously approved these changes Oct 28, 2020
View reviewed changes
@LouisEugeneMSFT LouisEugeneMSFT merged commit 1227d93 into main Oct 28, 2020
@LouisEugeneMSFT LouisEugeneMSFT deleted the leugene/issue4526 branch October 28, 2020 23:23
lei9444 added a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Oct 29, 2020
@lei9444 @beyackle
@cwhitten @yeze322 @hatpick @sorgh @taicchoumsft @a-b-r-o-w-n @LouisEugeneMSFT
Test skill (#2)
c401768 
* Update en-US.json

* bring in stuff from the draft branch

* make deletion work

* add error/warning icons

* read notification map for state

* fix type-checking and start on unit tests

* add sampleDialog and fix more tests

* add showAll

* rename to onAllSelected because it's a callback

* update unit tests

* fix onSelect handling in ProjectTree

* Update qna.test.tsx

* Update design.test.tsx

* add unit test

* fixes from PR comments

* restore multibots to tree

* fix links from project tree

* fix wrong IDs in URL

* yarn.lock rebuild

* update yarn.lock files

* fix action card links

* Update DesignPage.tsx

* fix unit test

* add unit test to ExpandableNode

* update ProjectTree unit tests

* Update ExpandableNode.tsx

* add rootProjectId to triggerApi

* fix creation and deletion bugs

* change mocks folder name

* Load design page only after skill project is loaded

Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>

* fixes from CR

* fix more tests

* fix: handle the case when selection is not empty (microsoft#4561)

Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>

* delete fd (microsoft#4556)

Co-authored-by: Soroush <sorgh@microsoft.com>

* fix more unit tests

* revert: bf-orchestrator lib integration (microsoft#4568)

This reverts commit 006a589.

Co-authored-by: Andy Brown <asbrown002@gmail.com>

* fix test typol

* ci: remove code scanning workflow (microsoft#4569)

* fix: Can not add properties in "SetProperties" action (microsoft#4572)

* bug fix

* pr comment

* pr comments

Co-authored-by: Ben Yackley <61990921+beyackle@users.noreply.github.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Co-authored-by: Srinaath Ravichandran <srravich@microsoft.com>
Co-authored-by: zeye <zeye@microsoft.com>
Co-authored-by: Soroush <hatpick@gmail.com>
Co-authored-by: Soroush <sorgh@microsoft.com>
Co-authored-by: taicchoumsft <61705609+taicchoumsft@users.noreply.github.com>
Co-authored-by: Andy Brown <asbrown002@gmail.com>
Co-authored-by: LouisEugeneMSFT <66701106+LouisEugeneMSFT@users.noreply.github.com>
@LouisEugeneMSFT LouisEugeneMSFT mentioned this pull request Nov 3, 2020
Closed
alanlong9278 added a commit to alanlong9278/BotFramework-Composer that referenced this pull request Nov 5, 2020
@alanlong9278
Merge branch 'settingsPage' into julong/publish-page
adc3543 
* settingsPage: (110 commits)
  code style
  update qna end point key after update qna subscription key
  handle comments
  handle comments
  chore: remove orchestrator (microsoft#4602)
  feat: taking control recognizer generation enhancements (microsoft#4549)
  chore: Set up Nightly static analysis pipeline in ADO (microsoft#4589)
  feature: show multi-skill project with correct link behavior (microsoft#4466)
  doc: guidelines/help-contents (microsoft#4354)
  Functions runtime: Revert to netcore3.0 for functions and make code more robust to functions variations (microsoft#4570)
  feat: Disable Connect to Skill menu action inside skill (microsoft#4557)
  fix (microsoft#4591)
  fix: Adds validation for form dialogs propery name (microsoft#4574)
  fix: update archive version in extensions/vacore (microsoft#4571)
  fix: e2e test (microsoft#4581)
  perf: avoid redraw when Project Tree selected item not changed (microsoft#4564)
  fix: security analysis failure due to bl package version (microsoft#4580)
  fix publish ut timeout (microsoft#4578)
  fix: Can not add properties in "SetProperties" action (microsoft#4572)
  ci: remove code scanning workflow (microsoft#4569)
  ...
lei9444 added a commit that referenced this pull request Nov 11, 2020
@lei9444 @beyackle
@cwhitten @yeze322 @hatpick @sorgh @taicchoumsft @a-b-r-o-w-n @LouisEugeneMSFT
feat: Diagnostics Page (#4457)
1ee2ad0 
* feat: Notifications Page

* update the unit tests

* fix merge conflict

* update the name

* fix unit test

* Test skill (#2)

* Update en-US.json

* bring in stuff from the draft branch

* make deletion work

* add error/warning icons

* read notification map for state

* fix type-checking and start on unit tests

* add sampleDialog and fix more tests

* add showAll

* rename to onAllSelected because it's a callback

* update unit tests

* fix onSelect handling in ProjectTree

* Update qna.test.tsx

* Update design.test.tsx

* add unit test

* fixes from PR comments

* restore multibots to tree

* fix links from project tree

* fix wrong IDs in URL

* yarn.lock rebuild

* update yarn.lock files

* fix action card links

* Update DesignPage.tsx

* fix unit test

* add unit test to ExpandableNode

* update ProjectTree unit tests

* Update ExpandableNode.tsx

* add rootProjectId to triggerApi

* fix creation and deletion bugs

* change mocks folder name

* Load design page only after skill project is loaded

Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>

* fixes from CR

* fix more tests

* fix: handle the case when selection is not empty (#4561)

Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>

* delete fd (#4556)

Co-authored-by: Soroush <sorgh@microsoft.com>

* fix more unit tests

* revert: bf-orchestrator lib integration (#4568)

This reverts commit 006a589.

Co-authored-by: Andy Brown <asbrown002@gmail.com>

* fix test typol

* ci: remove code scanning workflow (#4569)

* fix: Can not add properties in "SetProperties" action (#4572)

* bug fix

* pr comment

* pr comments

Co-authored-by: Ben Yackley <61990921+beyackle@users.noreply.github.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Co-authored-by: Srinaath Ravichandran <srravich@microsoft.com>
Co-authored-by: zeye <zeye@microsoft.com>
Co-authored-by: Soroush <hatpick@gmail.com>
Co-authored-by: Soroush <sorgh@microsoft.com>
Co-authored-by: taicchoumsft <61705609+taicchoumsft@users.noreply.github.com>
Co-authored-by: Andy Brown <asbrown002@gmail.com>
Co-authored-by: LouisEugeneMSFT <66701106+LouisEugeneMSFT@users.noreply.github.com>

* update url

* Revert "Test skill (#2)"

This reverts commit c401768.

* fix conflict

* fix e2e test

* fix unit test

* update the diagnostic page

* update the warning iconm

* fix lint

* add some unit tests

* fix conflict and update the navigation

* fix unit test

Co-authored-by: Ben Yackley <61990921+beyackle@users.noreply.github.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Co-authored-by: Srinaath Ravichandran <srravich@microsoft.com>
Co-authored-by: zeye <zeye@microsoft.com>
Co-authored-by: Soroush <hatpick@gmail.com>
Co-authored-by: Soroush <sorgh@microsoft.com>
Co-authored-by: taicchoumsft <61705609+taicchoumsft@users.noreply.github.com>
Co-authored-by: Andy Brown <asbrown002@gmail.com>
Co-authored-by: LouisEugeneMSFT <66701106+LouisEugeneMSFT@users.noreply.github.com>
@cwhitten cwhitten mentioned this pull request Nov 13, 2020
Merged
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
@LouisEugeneMSFT
fix: Can not add properties in "SetProperties" action (microsoft#4572)
fe32532 
* bug fix

* pr comment

* pr comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hatpick hatpick hatpick approved these changes

@GeoffCoxMSFT GeoffCoxMSFT GeoffCoxMSFT left review comments

@a-b-r-o-w-n a-b-r-o-w-n Awaiting requested review from a-b-r-o-w-n

@beyackle beyackle Awaiting requested review from beyackle

@boydc2014 boydc2014 Awaiting requested review from boydc2014

@cwhitten cwhitten Awaiting requested review from cwhitten

@srinaath srinaath Awaiting requested review from srinaath

@tonyanziano tonyanziano Awaiting requested review from tonyanziano

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can not add properties in "SetProperties" action
4 participants
@LouisEugeneMSFT @coveralls @hatpick @GeoffCoxMSFT