microsoft / botbuilder-dotnet

Welcome to the Bot Framework SDK for .NET repository, which is the home for the libraries and packages that enable developers to build sophisticated bot applications using .NET.
https://github.com/Microsoft/botframework
MIT License
869 stars 478 forks source link

schema:#/definitions/condition is treated the same as schema:#/definitions/booleanExpression #5334

Open tomlm opened 3 years ago

tomlm commented 3 years ago

Describe the bug

schema:#/definitions/condition indicates that the default UX for the property should be the expression editor. NOTE: It should NOT HAVE an equals.

schema:#/definitions/booleanExpression indicates that the value could be a bool TRUE/FALSE or an expression which evaluates to that.

If you look in the schema you can see that IfCondition action the condition is different then disabled

       "condition": {
            "$ref": "schema:#/definitions/condition",
            "title": "Condition",
            "description": "Expression to evaluate.",
            "examples": [
                "user.age > 3"
            ]
        },
        "disabled": {
            "$ref": "schema:#/definitions/booleanExpression",
            "title": "Disabled",
            "description": "Optional condition which if true will disable this action.",
            "examples": [
                true,
                "=user.age > 3"
            ]
        },

I have written 3 bots with composer and NOT ONCE have I had a trigger or IfCondition which was "TRUE/FALSE". 100% of the time it was an expression such as user.age > 3

The current UI is saying that the default for an If condition is true

   // writing a if statement like this almost never ever is done, because it's meaningless.
   if (true)

The effect of this bug is that I have to click 3 times EVERYTIME I create a new trigger or IfCondition.

  1. once to drop show menu
  2. once to select expression
  3. once to click on the input box. By the end of a long day I am swearing at the monitor.

Version

Browser

OS

To Reproduce

Steps to reproduce the behavior:

  1. Add a trigger with condition user.age > 3
  2. Add a IfCondition with condition user.name == 'john'

Expected behavior

I expect the schema design to be respected.

  1. Conditions should not be labeled as T/F and not have drop down menu. It should just be the expression editor without the '='.
  2. Boolean properties should have T/F and the drop as it does today.
  3. If I select expression for any expression property the focus should be placed on the input box so I don't have to have the third click.

This should be all places that /definitions/condition is used in the .schema files:

Screenshots

image

image

Additional context

cwhitten commented 3 years ago

@tomlm @GeoffCoxMSFT has some suggestions here for the SDK that will make this easier for Composer, and will follow-up

yeze322 commented 3 years ago

We have some findings

  1. condition fields are displayed as a dropdown with boolean because of sdk.schema declares it as oneOf field (generated by dialog:merge). image image

  2. booleanExpression is fine, it will be displayed as an abc string field in Composer (if edit sdk.schema manually) image image 67.png)

So for this problem, if we want to change all *.condition fields' behavior in Composer, it's better to update it in dotnet sdk as a thorough fix.

@cwhitten @boydc2014 FYI

cwhitten commented 3 years ago

Transferred to botbuildet-dotnet.

@tomlm we believe this should be fixed in schema. From our understanding of the problem and a way to address it is to have condition stop defining it's interface as a oneOf for expression or boolean. Because the schema says it can be a boolean, the form provides the related selectable options (TRUE/FALSE).