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

Adaptive cards #20

Merged
merged 20 commits into from
Jan 11, 2024
Merged

Adaptive cards #20

merged 20 commits into from
Jan 11, 2024

Conversation

kwinto
Copy link
Contributor

@kwinto kwinto commented Jan 9, 2024

This PR adds support for Adaptive Cards.

Also:

  • Improves demo page
  • Moves demo page to test to keep the code only relevant for the package in src

To test:

npm run dev

press 'o' + <Enter>

Note: there seem to be an issue related to Swiper+Adaptive Cards dependencies, it currently breaks test runs.

@kwinto kwinto requested review from sushmi21 and dshire January 9, 2024 20:04
@kwinto kwinto requested a review from fabclj January 10, 2024 08:21
@fabclj
Copy link
Contributor

fabclj commented Jan 10, 2024

I like the new demo page, but it would be great to integrate all messages types.
Also it would be great to solve the issue about swiper and adaptive card.

@fabclj
Copy link
Contributor

fabclj commented Jan 11, 2024

I have run the adaptive card on webchat locally. I list here the first findings:

  • Primary button should have height 44px and cursor: pointer
  • The card width should be max 295px like all messages
  • Textarea scrollbar is not styled as Figma requirments

Screenshot from 2024-01-11 12-11-03

package.json Show resolved Hide resolved
@kwinto
Copy link
Contributor Author

kwinto commented Jan 11, 2024

  • Primary button should have height 44px and cursor: pointer
    Fixed
  • The card width should be max 295px like all messages
    Full width for AC messages is specified in Figma
  • Textarea scrollbar is not styled as Figma requirments
    I fixed it as far as it is possible with CSS. We lack the possibility to have a margin and coloured "track" at the same time on a scrollbar.

Further improvements here will need a custom AC render or some kind of field override, as we are at the limit of what is possible with to do with pure CSS.

@kwinto
Copy link
Contributor Author

kwinto commented Jan 11, 2024

I like the new demo page, but it would be great to integrate all messages types. Also it would be great to solve the issue about swiper and adaptive card.

Tracked in new stories for the next sprint.

@fabclj
Copy link
Contributor

fabclj commented Jan 11, 2024

Text area now has scrollbar but is still not fitting the design requirements

Screenshot from 2024-01-11 16-20-25

@fabclj fabclj self-requested a review January 11, 2024 17:24
@kwinto kwinto merged commit 7927b3e into main Jan 11, 2024
0 of 2 checks passed
@kwinto kwinto deleted the adaptive-cards branch January 11, 2024 17:58
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.

2 participants