-
Notifications
You must be signed in to change notification settings - Fork 543
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
Feature/carousel #6724
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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
almedina-ms
requested review from
matthidinger,
jonmill,
jwoo-msft,
licanhua and
paulcam206
November 11, 2021 17:50
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. |
licanhua
reviewed
Nov 11, 2021
source/nodejs/adaptivecards-site/themes/adaptivecards/layout/_partial/head.ejs
Outdated
Show resolved
Hide resolved
source/nodejs/adaptivecards-site/themes/adaptivecards/layout/designer.ejs
Outdated
Show resolved
Hide resolved
* 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
almedina-ms
commented
Nov 11, 2021
* [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
Co-authored-by: nesalang <[email protected]>
jwoo-msft
approved these changes
Nov 12, 2021
* [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
paulcam206
approved these changes
Nov 13, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
licanhua
approved these changes
Nov 13, 2021
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Pagination
Autoplay
RTL
Sample Card
See folder samples/1.6
How Verified
The implementation was verified by
Microsoft Reviewers: Open in CodeFlow