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

[Rendering] [iOS] ACR button rendering Is wonky as the SDK sets up height constraint and so does the Teams app #6082

Closed
shabripabba opened this issue Jul 14, 2021 · 1 comment · Fixed by #6158
Assignees
Labels
Area-Renderers Bug Partner-MsftTeams Issues related to AC integration with Microsoft Teams partner

Comments

@shabripabba
Copy link
Contributor

Target Platform

Please list the platform(s) that your application is targeting and remove platforms you are not targeting:

  • iOS

Target Application

AC iOS SDK v 1.5, pod 2.8.0-beata1

Please provide the name of the application that is rendering the card (ex: Outlook, Teams, etc)
Teams

Problem Description

When upgrading from 2.4.6 AC SDK to 2.8.0-beata1. It looks like the ACR button being used on Teams SDK gets rendered with the text cutoff, and also the button returns a wrong "Intrinsic Size", which is what the Teams SDK uses to give the button a width constraint.

Mobile Platform Bellevue is working on upgrading the current AC SDK on Teams to v1.5 which is branch release/21.04 and the pod version is 2.8.0-beata1.

There are a couple things

  1. We noticed that there have been changes in the ACRButton adds a height constraint to the ACRButton

[button.heightAnchor constraintGreaterThanOrEqualToAnchor:button.titleLabel.heightAnchor constant:button.contentEdgeInsets.top + button.contentEdgeInsets.bottom].active = YES;

And every custom renderer we have on the Teams app, uses ACRButton and then adds our own heigh Constraint, now these two constrains clash and make the button appear weirdly(Screenshots attached)

  1. We also noticed that the ACRButtonExpandable.xib and ACRButton.xib have been changed recently and the Teams app actually uses "button.titleLabel.IntrinsicSize" to assign a width constrain to the button for every customrender and the change made in the AC SDK for these xibs(Or the new constraints) return the intrinsicSize to be wrong. So this ends up making the button look wonky.

**The intrinsicSize issue could be potentially fixed on the Teams app relying on the titleLabel.text which would fix the buttons to show the entire text horizontally.

But the vertical part, since the constraints are clashing if it were to be fixed on Teams app, then we'd need to employ a hacky way of removing Height constraints on the ACRButton.**

Please enter a description of the issue. If you just have a question, please post on StackOverflow instead

Screenshot

Please provide a screenshot of the problem (if possible)
Screen Shot 2021-07-06 at 11 02 22 AM

Card JSON

{"type":"AdaptiveCard","version":"1.0","body":[{"type":"RichTextBlock","inlines":[{"type":"TextRun","text":"Please select your country code from the drop down and enter your number in the field below (no special characters allowed) ","wrap":true,"size":"Small","color":"Accent"},{"type":"TextRun","text":" *","color":"Attention","weight":"Bolder"}],"isVisible":true},{"type":"ColumnSet","columns":[{"type":"Column","width":"stretch","items":[{"type":"Input.ChoiceSet","placeholder":"Pick a country code","choices":[{"title":"Choice 1","value":"Choice 1"},{"title":"Choice 2","value":"Choice 2"}],"id":"country"}]},{"type":"Column","width":"stretch","items":[{"type":"Input.Text","placeholder":"Type the number here","id":"NewPhoneNumber"}]}],"isVisible":true},{"type":"RichTextBlock","inlines":[{"type":"TextRun","text":"Set privacy ","wrap":true,"size":"Small","color":"Accent"},{"type":"TextRun","text":" *","color":"Attention","weight":"Bolder"}],"isVisible":true},{"type":"Input.ChoiceSet","placeholder":"Placeholder text","choices":[{"title":"This is json sample for the Test)","value":"isPrimary"},{"title":"This is json sample for the Test","value":"ispublic"}],"style":"expanded","wrap":true,"spacing":"Small","id":"Privacy","value":"","isVisible":true},{"type":"ActionSet","actions":[{"type":"Action.Submit","title":"Submit","data":{"Action":"Test"}},{"type":"Action.Submit","title":"Cancel","data":{"Action":"cancel"}}],"isVisible":true},{"type":"TextBlock","text":"Please refer to the following rules and enter the correct number above.","wrap":true,"color":"Attention","isVisible":false},{"type":"FactSet","facts":[{"title":"Allowed no. of digits","value":"{AllowedDigit}"}],"isVisible":false,"separator":true},{"type":"TextBlock","text":"No special characters allowed.","isVisible":false,"weight":"Bolder"},{"type":"TextBlock","text":"Please fill all mandatory fields above and 'Submit' again.\n","wrap":true,"color":"Attention","isVisible":false},{"type":"TextBlock","text":"Please enter the correct format above and try again.","wrap":true,"color":"Attention","isVisible":false}],"$schema":"http://adaptivecards.io/schemas/adaptive-card.json"}

Sample Code

If you add
[button.heightAnchor constraintEqualToConstant:TSkButtonHeight].active = YES;

after line number 36 in ACRActionShowCardRenderer.mm file you should see the issue as well

Version of SDK

What version are you using? Ex: NuGet 1.0.2, or latest main, etc...
release/21.04 branch and pod version 2.8.0-beata1

@jonmill jonmill added Partner-MsftTeams Issues related to AC integration with Microsoft Teams partner Triage-Approved for Fix and removed Triage-Needed labels Jul 15, 2021
@jonmill jonmill added this to the 21.08 milestone Jul 26, 2021
@jwoo-msft
Copy link
Member

@shabripabba
For 1st issue, the height constraint can become a part of the public interface for the button, so you have to remove it if you want.
For 2nd issue, calculating the buttons' intrinsic content size solely based on the tile is wrong. you also have to take into account the image and the edge insets.
c.c @jonmill

@ghost ghost added the Status-In Code Review label Aug 3, 2021
jwoo-msft added a commit that referenced this issue Aug 6, 2021
* [iOS] updated action test

* [iOS] Added vertical constraint to public API and mocks for unit testing

* [iOS] added title width constraint to public api
@jonmill jonmill added this to To do in Current Sprint via automation Aug 19, 2021
@jonmill jonmill moved this from To do to Done in Current Sprint Aug 19, 2021
michaelfarnsworth pushed a commit to michaelfarnsworth/AdaptiveCards that referenced this issue Nov 10, 2022
* [iOS] updated action test

* [iOS] Added vertical constraint to public API and mocks for unit testing

* [iOS] added title width constraint to public api
rankush pushed a commit to rankush/AdaptiveCards that referenced this issue May 8, 2024
* [iOS] updated action test

* [iOS] Added vertical constraint to public API and mocks for unit testing

* [iOS] added title width constraint to public api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Renderers Bug Partner-MsftTeams Issues related to AC integration with Microsoft Teams partner
Projects
Development

Successfully merging a pull request may close this issue.

3 participants