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

Feature/carousel #6724

Merged
merged 30 commits into from
Nov 13, 2021
Merged

Feature/carousel #6724

merged 30 commits into from
Nov 13, 2021

Conversation

almedina-ms
Copy link
Contributor

@almedina-ms almedina-ms commented Nov 11, 2021

Related Issue

Fixes #6438

Description

Implement the Carousel control as part of the 1.6 schema for the JS renderer.

Changes include parsing, rendering, unit tests and UI tests

Navigatioin

carousel1

Pagination

pagination

Autoplay

autoplay

RTL

rtl

Sample Card

See folder samples/1.6

How Verified

The implementation was verified by

  • Unit testing
  • UI testing
  • Manual testing and visual verification
Microsoft Reviewers: Open in CodeFlow

almedina-ms and others added 21 commits November 9, 2021 14:05
* added sample

* Add serialization property

* sample update

* TBD property parsing working

* Parse CarouselPage object

* Code that adds containers

* added js

* [compilation fix]

* fixed issue

* Add todo list

* Fix extra spacing

* Show first element on start up

* implemented toDo items in toDo.md

* updated dot ui

* updated dot

* added a new scenario card

* Version not yet working

* Version almost working

* Hard code sample

* package changes

* Add more modifications that I saw on the interwebz

* fixed swiper

* Version where some stuff works but other stuff doesnt

* updated carousel render

* [update]

* Fix carousel

* Fix more stuff

* Version that works!! Yeah!!

* added walkaround for css loading issue

* added carousel cards to ui tests

* Fix more stuff

* Fix arrows to look better

* changed swiper properties from static class member variable to private member variable

* Fix static variables

* refactored

* fixed tab

* updated

Co-authored-by: nesalang <[email protected]>
Co-authored-by: Paul Campbell (DEP) <[email protected]>
* Rename TBD to display

* Undo changes to site
* Rename TBD to display

* Undo changes to site

* Rename unsupported elements list

* Fix parsing to handle nested items

* Upgrade sample

* Remove commented code

* Handle forbidden actions

* Update test to handle containers actions
* updated for accessibility

* reverted Action.OpenURL changes
* updated for accessibility

* reverted Action.OpenURL changes

* added host config options

* fixed a11y tab index issue not being able to use space on pagination
bulltes

* Revert "fixed a11y tab index issue not being able to use space on pagination"

This reverts commit 16da572.
…bullets (#6677)

* updated for accessibility

* reverted Action.OpenURL changes

* added host config options

* fixed a11y tab index issue not being able to use space on pagination
bulltes

* updated it

* clean-up
* Fix pause autoplay on hover

* Fix build break
* Add unit test for carousel page

* Remove swiper from root level import

* Add swiper as dev dependency
* added carousel page id upon action invocation

* review comments addressed
* [JS] Carousel schema changes

* Pass `forbiddenTypes` instead of the empty set during fallback

* Remove `getPage[At|Count]()` as they conflict with mandatory `getItem[At|Count]()`
* Rename TBD to display

* Undo changes to site

* Rename unsupported elements list

* Fix parsing to handle nested items

* Upgrade sample

* Remove commented code

* Handle forbidden actions

* Update test to handle containers actions

* Improve css styles to have a better control

* Remove changes made to designer

* Fix cards not showing up

* Fix merge

* Fix css again

* Remove important flags

* Fix issue where keyboard navigation goes to other cards

* Fix sample

* Add semicolon
Just a minor oversight - the version of `jest` I saw recommended to fix our problems consuming the `swiper` module ended
up being a pre-release version. `jest@^27` has since had a full release, so I just wanted to make sure we were requiring
it specifically. For what it's worth, we weren't actually *using* the prerelease version see
[here](https://github.com/microsoft/AdaptiveCards/blob/2c3f8530556213a1a1f5ed742b62c28f24a4c24d/source/nodejs/adaptivecards/package-lock.json#L4485-L4486),
but all the same it's better to declare our dependency against full-fledged release.
* updatred supported properties for Carousel & CarouselPages

* updated spacing in sample
* Update Tests for timer vs no timer

* Fix autoplay being instantiated without being required

* Fixes and some ui tests

* Fix another json file and add working tests

* Fix autoplay issue

* Add more tests

* Format json

* Fix PR comments

* Fix carousel again and fix test

* Refactor some code
@ghost
Copy link

ghost commented Nov 11, 2021

Hi @almedina-ms. Thanks for helping make the AdaptiveCards JS renderer + tooling better. As additional verification, once the JS build succeeds, please go to the test site to test out your website/designer changes.

* Changed teams target version to 1.4

* fixed blog link to point to proper release

* Added horizontal carousel spec

Co-authored-by: Paul Campbell <[email protected]>
* Pull css changes to ui test app

* Make cards in carousel use as much space as possible
jwoo-msft and others added 4 commits November 11, 2021 22:16
* [JS][Carousel] Fixed sudden jump of carousel page when focused

* [JS][Carousel] Updated RTL

* added a minor changes

* [JS][Carousel] Made RTL workw with navigation css changes

* change per review comments

* Revert "change per review comments"

This reverts commit 3ad698b.

* clean change commit
* [JS][Carousel] Updated SubmitAction

* review coment

* updated hostconfig json
* [JS] Change how we produce/consume CSS

See #6710 -- this is a partial solution

* Fix indenting

* Back out case-insensitivity in regex

* Remove `sass-loader` dependency

* Fixes for `adaptivecards-ui-testapp`

* Fix quoting on `generate-css` script

Fix build break on Windows?

* Fix up `adaptivecards-designer-app`

* Fix up `adaptivecards-react`

* Fix `webpack.config.js` formatting
Copy link
Member

@paulcam206 paulcam206 left a comment

Choose a reason for hiding this comment

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

:shipit:

@paulcam206 paulcam206 enabled auto-merge (squash) November 13, 2021 01:51
@paulcam206 paulcam206 merged commit 65de1ad into main Nov 13, 2021
@paulcam206 paulcam206 deleted the feature/carousel branch November 13, 2021 02:27
michaelfarnsworth pushed a commit to michaelfarnsworth/AdaptiveCards that referenced this pull request Nov 10, 2022
* Merge prototype branch (microsoft#6643)

* added sample

* Add serialization property

* sample update

* TBD property parsing working

* Parse CarouselPage object

* Code that adds containers

* added js

* [compilation fix]

* fixed issue

* Add todo list

* Fix extra spacing

* Show first element on start up

* implemented toDo items in toDo.md

* updated dot ui

* updated dot

* added a new scenario card

* Version not yet working

* Version almost working

* Hard code sample

* package changes

* Add more modifications that I saw on the interwebz

* fixed swiper

* Version where some stuff works but other stuff doesnt

* updated carousel render

* [update]

* Fix carousel

* Fix more stuff

* Version that works!! Yeah!!

* added walkaround for css loading issue

* added carousel cards to ui tests

* Fix more stuff

* Fix arrows to look better

* changed swiper properties from static class member variable to private member variable

* Fix static variables

* refactored

* fixed tab

* updated

Co-authored-by: nesalang <[email protected]>

* Initial design (microsoft#6644)

Co-authored-by: Paul Campbell (DEP) <[email protected]>

* Prototype clean up (microsoft#6651)

* Rename TBD to display

* Undo changes to site

* removed cns css from header.ejs and removed css from prototype 0 (microsoft#6653)

* Fix support for unsupported elements (microsoft#6654)

* Rename TBD to display

* Undo changes to site

* Rename unsupported elements list

* Fix parsing to handle nested items

* Upgrade sample

* Remove commented code

* Handle forbidden actions

* Update test to handle containers actions

* Add support for autoplay (microsoft#6665)

* Jwoo/feature/carousel 2 (microsoft#6673)

* updated for accessibility

* reverted Action.OpenURL changes

* [Carousel] Added Host Config options (microsoft#6675)

* updated for accessibility

* reverted Action.OpenURL changes

* added host config options

* fixed a11y tab index issue not being able to use space on pagination
bulltes

* Revert "fixed a11y tab index issue not being able to use space on pagination"

This reverts commit 16da572.

* fixed a11y tab index issue not being able to use space on pagination bullets (microsoft#6677)

* updated for accessibility

* reverted Action.OpenURL changes

* added host config options

* fixed a11y tab index issue not being able to use space on pagination
bulltes

* updated it

* clean-up

* Fix pause autoplay on hover (microsoft#6671)

* Fix pause autoplay on hover

* Fix build break

* Fix package.json files (microsoft#6678)

* Add unit test for carousel page

* Remove swiper from root level import

* Add swiper as dev dependency

* Added carousel page id (microsoft#6690)

* added carousel page id upon action invocation

* review comments addressed

* [JS] Carousel schema changes (microsoft#6667)

* [JS] Carousel schema changes

* Pass `forbiddenTypes` instead of the empty set during fallback

* Remove `getPage[At|Count]()` as they conflict with mandatory `getItem[At|Count]()`

* [Styling] Update swiper style (microsoft#6664)

* Rename TBD to display

* Undo changes to site

* Rename unsupported elements list

* Fix parsing to handle nested items

* Upgrade sample

* Remove commented code

* Handle forbidden actions

* Update test to handle containers actions

* Improve css styles to have a better control

* Remove changes made to designer

* Fix cards not showing up

* Fix merge

* Fix css again

* Remove important flags

* Fix issue where keyboard navigation goes to other cards

* Fix sample

* Add semicolon

* Fix addeventlisteners and focus not firing (microsoft#6704)

* [JS] Pin newer jest dependency for `adaptivecards` (microsoft#6702)

Just a minor oversight - the version of `jest` I saw recommended to fix our problems consuming the `swiper` module ended
up being a pre-release version. `jest@^27` has since had a full release, so I just wanted to make sure we were requiring
it specifically. For what it's worth, we weren't actually *using* the prerelease version see
[here](https://github.com/microsoft/AdaptiveCards/blob/2c3f8530556213a1a1f5ed742b62c28f24a4c24d/source/nodejs/adaptivecards/package-lock.json#L4485-L4486),
but all the same it's better to declare our dependency against full-fledged release.

* Jwoo/feature/remove properties (microsoft#6706)

* updatred supported properties for Carousel & CarouselPages

* updated spacing in sample

* Updated package-lock.json after rebase

* Fix CSS story for now

* UI Tests for carousel (microsoft#6707)

* Update Tests for timer vs no timer

* Fix autoplay being instantiated without being required

* Fixes and some ui tests

* Fix another json file and add working tests

* Fix autoplay issue

* Add more tests

* Format json

* Fix PR comments

* Fix carousel again and fix test

* Refactor some code

* Add timeout (microsoft#6718)

* Carousel spec (microsoft#6698)

* Changed teams target version to 1.4

* fixed blog link to point to proper release

* Added horizontal carousel spec

Co-authored-by: Paul Campbell <[email protected]>

* Fix minor Css changes for carousel (microsoft#6722)

* Pull css changes to ui test app

* Make cards in carousel use as much space as possible

* [JS][Carousel] Fixed sudden jump of carousel page when focused (microsoft#6720)

* Updated RTL support for Carousel (microsoft#6723)

* [JS][Carousel] Fixed sudden jump of carousel page when focused

* [JS][Carousel] Updated RTL

* added a minor changes

* [JS][Carousel] Made RTL workw with navigation css changes

* change per review comments

* Revert "change per review comments"

This reverts commit 3ad698b.

* clean change commit

* [JS][Carousel] Updated SubmitAction (microsoft#6728)

* [JS][Carousel] Updated SubmitAction

* review coment

* updated hostconfig json

* Fix PR comments (microsoft#6727)

Co-authored-by: nesalang <[email protected]>

* [JS] Change how we produce/consume CSS (microsoft#6732)

* [JS] Change how we produce/consume CSS

See microsoft#6710 -- this is a partial solution

* Fix indenting

* Back out case-insensitivity in regex

* Remove `sass-loader` dependency

* Fixes for `adaptivecards-ui-testapp`

* Fix quoting on `generate-css` script

Fix build break on Windows?

* Fix up `adaptivecards-designer-app`

* Fix up `adaptivecards-react`

* Fix `webpack.config.js` formatting

Co-authored-by: nesalang <[email protected]>
Co-authored-by: Paul Campbell (DEP) <[email protected]>
Co-authored-by: J.P. Roca <[email protected]>
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.

[Feature Request] Carousel Card Support
5 participants