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

[JavaScript][Input.Number] [Type of value has changed with latest update] #5111

Closed
1 task done
aroraketan opened this issue Nov 26, 2020 · 12 comments · Fixed by #5134
Closed
1 task done

[JavaScript][Input.Number] [Type of value has changed with latest update] #5111

aroraketan opened this issue Nov 26, 2020 · 12 comments · Fixed by #5134
Labels
Area-Inconsistency Bugs around renderer inconsistencies across different platforms Bug Partner-MsftTeams Issues related to AC integration with Microsoft Teams partner
Milestone

Comments

@aroraketan
Copy link

aroraketan commented Nov 26, 2020

Platform

What platform is your issue or question related to? (Delete other platforms).

  • JavaScript

Author or host

Host: Microsoft Teams (multi-window).

Version of SDK

JS v2.5.0.

Details

The value prop of Input.Number has changed from string type in v1.2.5 to number | undefined type in v2.x.
This can potentially break existing host scenarios.
image

@aroraketan aroraketan added the Bug label Nov 26, 2020
@ghost ghost added the Triage-Needed label Nov 26, 2020
@ghost ghost added this to the 21.01 milestone Nov 26, 2020
@ghost ghost added the Area-Inconsistency Bugs around renderer inconsistencies across different platforms label Nov 26, 2020
@ghost
Copy link

ghost commented Nov 26, 2020

please review this issue for target Milestone, Inconsistencies & Priority upon triage.

@ghost ghost added the High Priority label Nov 26, 2020
@shalinijoshi19
Copy link
Member

@dclaux isn't this related to the Input.Number string vs int discussions we'd been having as part of #3287 earlier this year.. I know we were going to modify the code to accept both number and string types for backwards compatibility. Is that something that is potentially addressed with #5093 ?

@ghost ghost removed the Triage-Needed label Nov 30, 2020
@ghost
Copy link

ghost commented Nov 30, 2020

Hi @aroraketan. We have acknowledged this issue report. Please continue to follow this issue for updates/progress/questions.

@ghost ghost added the Triage-Investigate label Nov 30, 2020
@shalinijoshi19 shalinijoshi19 added 1.3 Upgrade Blocker Partner-MsftTeams Issues related to AC integration with Microsoft Teams partner and removed Triage-Investigate labels Nov 30, 2020
@shalinijoshi19 shalinijoshi19 modified the milestones: 21.01, 20.11-12 Nov 30, 2020
@ghost ghost added the AdaptiveCards v20.11 label Nov 30, 2020
@dclaux
Copy link
Member

dclaux commented Nov 30, 2020

@aroraketan @shalinijoshi19 this is by design. The behavior was changed because the JS SDK's behavior didn't match that of the other SDKs.

@shalinijoshi19 shalinijoshi19 assigned paulcam206 and unassigned dclaux Nov 30, 2020
@shalinijoshi19
Copy link
Member

@paulcam206 to pick this up to address the breaking change aspect of the original change for JS.

@paulcam206
Copy link
Member

also need to update schema and enumerate proximal changes

@dclaux
Copy link
Member

dclaux commented Nov 30, 2020

@shalinijoshi19 what is there to address?

@shalinijoshi19
Copy link
Member

Referencing #3829 which was the issue filed by the BotFramework team around the same issue. @paulcam206 / @dclaux FYI.

@shalinijoshi19
Copy link
Member

shalinijoshi19 commented Dec 4, 2020

@aroraketan Input.Number is expected to be a number type per our schema.

  • Infact we had inadvertently broken the 1.2.x series at some point to switch from accepting Input.Number as number only in 1.2.0 to string starting likely with 1.2.3, causing a botframework scenario break as reported in Should Input.Number value accept a string? #3287 and leading to an inconsistency in the JS renderer;
  • We've since addressed that discrepancy in the JS renderer and the 2.x series now matches our schema and other renderers around Input.Number handling.

@dclaux pls correct me if i am wrong here
@paulcam206 FYI; Doesnt appear we have anything to do here.

@shalinijoshi19
Copy link
Member

@sowrabh-msft FYI..

@shalinijoshi19
Copy link
Member

@aroraketan Input.Number is expected to be a number type per our schema.

  • Infact we had inadvertently broken the 1.2.x series at some point to switch from accepting Input.Number as number only in 1.2.0 to string starting likely with 1.2.3, causing a botframework scenario break as reported in Should Input.Number value accept a string? #3287 and leading to an inconsistency in the JS renderer;
  • We've since addressed that discrepancy in the JS renderer and the 2.x series now matches our schema and other renderers around Input.Number handling.

@dclaux pls correct me if i am wrong here
@paulcam206 FYI; Doesnt appear we have anything to do here.

@aroraketan / @sowrabh-msft updates on this one:
So per our offline conversation on this one, it turned out not about the actual Input.Number schema but the transformed callback data type that is passed in to the host Submit.Action callbacks in case of Input.Number here and the type of that object that was divergent and inconsistent with other platforms on JS.
This inconsistency has now been reverted and JS back to passing the Input.Number as a string to the Action.Submit callback in the host.
Meanwhile longer term we realize it's an awkward roundtripping story and we are tracking addressing that experience with #5135 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Inconsistency Bugs around renderer inconsistencies across different platforms Bug Partner-MsftTeams Issues related to AC integration with Microsoft Teams partner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants