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

feat: Update Composer logic to leverage the uiSchema when initializing the IntellisenseField. #4263

Merged
merged 7 commits into from
Sep 29, 2020

Conversation

LouisEugeneMSFT
Copy link
Contributor

@LouisEugeneMSFT LouisEugeneMSFT commented Sep 24, 2020

Description

This is the 3rd and final step to make Composer leverage the uiSchema for Intellisense mappings between components and options (for example "Intellisense for Microsoft.DeleteProperty should show user variables").

Step 1 was modifying the schema of the uiSchema: microsoft/botframework-sdk#6047
Step 2 was updating the uiSchema of individual components: microsoft/botbuilder-dotnet#4704

This PR is removing the hardcoded mappings and injecting the projectId into IntellisenseTextField to make it work with user variables.

Task Item

fixes #4188

Screenshots

image

@@ -35,13 +33,7 @@ export const getRowProps = (rowProps: FormRowProps, field: string) => {
const { required = [] } = schema;
const fieldSchema = resolvePropSchema(schema, field, definitions);

const intellisenseScopes: string[] = [];
if (field === 'property') {
intellisenseScopes.push('variable-scopes');
Copy link
Member

Choose a reason for hiding this comment

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

I see the logic is deleted from FormRow, but where is it added in IntellisenseField? I only see the use of projectId in IntellisenseField.tsx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @GeoffCoxMSFT, I made sure that Intellisense can consume the data from the uiSchema in the 1st Intellisense PR.
This is what uiOptions.properties?.[field] does.
I only needed the hardcoding as a temporary thing until the data was in the schema.

Copy link
Contributor

Choose a reason for hiding this comment

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

One thing to be aware of is that users with old ui schemas will not get this behavior. Maybe we keep the hardcoded path for now, but prefer the UI Schema options if they are set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the uiSchema not bundled with the code? Aka won't users with the latest app version not also have the latest uiSchema? I would rather avoid any hardcoding when we can. Any field with the "property" value will get one type of Intellisense, which might lead to unexpected behaviors if that value is used for a different purpose (aka not a variable setter) somewhere in the schema.

@coveralls
Copy link

coveralls commented Sep 25, 2020

Coverage Status

Coverage decreased (-0.006%) to 55.651% when pulling 115c3e4 on leugene/usingUISchemaForIntellisense into 9852341 on main.

@a-b-r-o-w-n a-b-r-o-w-n self-assigned this Sep 25, 2020
@LouisEugeneMSFT LouisEugeneMSFT merged commit 1b2b274 into main Sep 29, 2020
@LouisEugeneMSFT LouisEugeneMSFT deleted the leugene/usingUISchemaForIntellisense branch September 29, 2020 00:27
@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
…g the IntellisenseField. (microsoft#4263)

* using only ui schema for mappings

* lint

Co-authored-by: Geoff Cox (Microsoft) <gcox@microsoft.com>
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.

Update Composer logic to leverage the uiSchema when initializing the IntellisenseField.
4 participants