-
Notifications
You must be signed in to change notification settings - Fork 551
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
Updated Carousel and ImageSet spec #8220
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
# Carousel support for Adaptive Cards | ||
|
||
To help drive the discussion and requirements around the carousel element, @rebecch and I sat down to go over how to meet our goals with a narrow but expandable set of schema changes. | ||
|
||
## Original Proposal | ||
We propose that carousels are a special type of Adaptive Card rather than an element. This greatly simplifies the design from both a schema perspective and a rendering perspective. In particular: | ||
* **We don't have to worry about nesting carousel elements.** | ||
* It's unclear how we could have a rational display of *N* nested carousels. | ||
|
@@ -11,61 +10,52 @@ We propose that carousels are a special type of Adaptive Card rather than an ele | |
* **We can ignore `Action.ToggleVisibility`.** | ||
|
||
## Spec By Example | ||
|
||
It's probably best to lead with a proposed sample carousel card... | ||
|
||
```json | ||
{ | ||
"$schema": "http://adaptivecards.io/schemas/adaptive-card.json", | ||
"type": "AdaptiveCard", | ||
"version": "1.6", | ||
"TBD": { | ||
"type": "carousel", | ||
"timer": "5000ms", | ||
"someOtherCarouselProperty": true | ||
}, | ||
"body": [ | ||
{ | ||
"type": "CarouselPage", | ||
"id": "firstCarouselPage", | ||
"selectAction": { | ||
"type": "Action.OpenUrl", | ||
"title": "Click for more information about the first carousel page!", | ||
"url": "https://adaptivecards.io/" | ||
}, | ||
"items": [ | ||
{ | ||
"type": "TextBlock", | ||
"weight": "bolder", | ||
"size": "large", | ||
"text": "This is the first carousel page, and it's *awesome*!" | ||
}, | ||
{ | ||
"type": "Container", | ||
"items": [ | ||
{} | ||
] | ||
} | ||
] | ||
}, | ||
{ | ||
"type": "CarouselPage", | ||
"id": "theSecondCarouselPage", | ||
"items": [ | ||
{ | ||
"type": "TextBlock", | ||
"text": "Welcome to page 2!" | ||
} | ||
] | ||
}, | ||
"body": | ||
{ | ||
"type": "CarouselPage", | ||
"id": "last-carousel-page", | ||
"items": [ | ||
"type": "Carousel", | ||
"timer": 5000, | ||
"pages":[ | ||
{ | ||
"type": "Image", | ||
"url": "https://adaptivecards.io/content/cats/3.png", | ||
"altText": "That's a cool cat!" | ||
{ | ||
"type": "CarouselPage", | ||
"id": "firstCarouselPage", | ||
"selectAction": { | ||
"type": "Action.OpenUrl", | ||
"title": "Click for more information about the first carousel page!", | ||
"url": "https://adaptivecards.io/" | ||
}, | ||
"items": [ | ||
{ | ||
"type": "TextBlock", | ||
"weight": "bolder", | ||
"size": "large", | ||
"text": "This is the first carousel page, and it's *awesome*!" | ||
}, | ||
{ | ||
"type": "Container", | ||
"items": [ | ||
{} | ||
] | ||
} | ||
] | ||
}, | ||
{ | ||
"type": "CarouselPage", | ||
"id": "theSecondCarouselPage", | ||
"items": [ | ||
{ | ||
"type": "TextBlock", | ||
"text": "Welcome to page 2!" | ||
} | ||
] | ||
} | ||
} | ||
] | ||
} | ||
|
@@ -80,25 +70,24 @@ It's probably best to lead with a proposed sample carousel card... | |
} | ||
``` | ||
|
||
So here we have a sample carousel card. You'll probably notice the ominously-named `TBD` property right away -- we haven't come up with a name for it yet. At any rate, this property is an object containing a `type` property describing what type of card this is. Other properties in this object can be provided to configure behaviors of this type of card (in this example, the timer to use for automatically flipping through the items in a carousel). | ||
|
||
You'll probably also notice the new `CarouselPage` element. It acts pretty similarly to a `Container`, but with some differences: | ||
So here we have a sample carousel card. You'll probably notice the new `CarouselPage` element. It acts pretty similarly to a `Container`, but with some differences: | ||
* No `style` (or at least, we don't have a reason to have `style` here yet) | ||
* No `bleed` | ||
* We don't allow every element type within a `CarouselPage`, only the subset we decide on (see following sections for more details) | ||
|
||
Having this new element also has the advantage of allowing us to reuse it later should we decide to promote the idea of `Carousel` as a regular page element (a `Carousel` would be a collection of `CarouselPage`s in this regime, though we'd need to decide how to reconcile `TBD` against per-`Carousel` settings). | ||
Having this new element also has the advantage of allowing us to reuse it later should we decide to promote the idea of `Carousel` as a regular page element (a `Carousel` would be a collection of `CarouselPage`s in this regime. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing closing ')' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
This sample also has a page-wide `Action.Execute`. When the user clicks this action (rendered as a button below the ⚬●⚬⚬ control), the host will receive the standard callback. The `id` of the currently-visible page should be exposed as a property on the `Action` the callback provides (we may also want to provide positional properties as well, but I'm not sure if they're needed). | ||
|
||
It's worth noting that we think we *should* allow `ActionSet` elements inside a `CarouselPage`, but they should be restricted to the same actions as the toplevel `actions` property. | ||
It's worth noting that we think we *should* allow `ActionSet` elements inside a `CarouselPage`, but they should be restricted to the same actions as the top level `actions` property. | ||
|
||
Another thing to note is that this implies the existence of a special carousel card inside of an `Action.ShowCard`, which we think is probably okay, but we should talk it through a bit more. If we do allow it, input gathering should happen as it currently does -- namely, the parent card's inputs are supplied if the `Action.ShowCard` has an `Action.Submit` or `Action.Execute` invoked from within it. | ||
|
||
## Disallowed Elements | ||
|
||
In order to meet our schedule, we need to avoid some common pain points that would require extra dev/spec time that might not necessarily lead to a better card author or AC host experience. In particular, we believe that the following scenarios/experiences should be explicitly not supported for v1: | ||
|
||
* Carousel | ||
* ShowCard | ||
* ToggleVisibility | ||
* Input Elements | ||
|
@@ -108,54 +97,55 @@ In order to meet our schedule, we need to avoid some common pain points that wou | |
|
||
All elements not mentioned above are allowed inside a carousel. | ||
|
||
## Header | ||
![image](https://user-images.githubusercontent.com/4112696/183519757-156a18a9-73e7-47d9-8e00-ad84d0070f99.png) | ||
## Timer Property | ||
* `"timer" : number` | ||
* when set, the number sets the duration before a Carousel page transitions to the next Carousel page. | ||
* while hovering, mouse click, and touch events, this auto transition is canceled. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will the timer start again after the manual transition has been completed? |
||
|
||
Header is an optional property whose value is an array of TextBlocks. Header is positioned above the carousel page, and the position of the text is static. | ||
When carousel page transitions to a new page, the text shall remain. Each subsequent `TextBlock` is laid below a previous `TextBlock` as `TextBlock`s in `body` of `AdaptiveCards` would be rendered. | ||
## HostConfig Options | ||
* Max number of `CarouselPage`s | ||
* Minimum timer restriction | ||
* Do we need host config theming? We're leaning towards leaving this for native styling via CSS. | ||
|
||
## Updated Proposal | ||
There had been strong demand to adorn the Carousel with other AdaptiveCard elements as shown in the image below. | ||
Customers wanted to have an option that allow these decorative elements around the Carousel remain static when CarouselPage is changed. | ||
Customers also wanted to have conditional layout template that can be switched between standard layout and layout with Carousel. | ||
Thus, we have relaxed the rule of treating Carousel as a special type of AdaptiveCard, and made Carousel as a regular AdaptiveCard element while all other rules remain intact. | ||
|
||
Header can be used for purely decorative purpose as well as a11y header. | ||
Author of card can designate a TextBlock in the array to be a11y header by setting its `style` property to `heading`. | ||
Renderers shall apply a11y heading role according to their respective a11y options. | ||
![image](https://user-images.githubusercontent.com/4112696/183519757-156a18a9-73e7-47d9-8e00-ad84d0070f99.png) | ||
|
||
Header supports an implied type. The implied type of `header` is `TextBlock`. | ||
As a result, author can shortcircuits specifying the type. When this happens, renderers shall assume the type `TextBlock` and renders the content accordingly. | ||
Implied type shall be remained `TextBlock` even when additional types are added to the list of supported types in the future. | ||
``` | ||
{ | ||
"type": "Carousel", | ||
"header": [ | ||
{ | ||
"text": "cat picture", | ||
"isSubtle": true, | ||
"size": "small" | ||
}, | ||
{ | ||
"text": "Top Cat Picture", | ||
"weight": "bolder", | ||
"style": "heading, | ||
"size": "large" | ||
}, | ||
{ | ||
"text": "Washington", | ||
"isSubtle": true | ||
} | ||
], | ||
"pages": | ||
... | ||
} | ||
Snippet of json of the above image | ||
```json | ||
"body": [ | ||
{ | ||
"type": "Container", | ||
"items": [ | ||
{ | ||
"type": "TextBlock", | ||
"text": "Photo Album", | ||
"size": "ExtraLarge", | ||
"style": "heading", | ||
"horizontalAlignment": "Center" | ||
}, | ||
{ | ||
"type": "TextBlock", | ||
"text": "Landscapes", | ||
"size": "Large", | ||
"horizontalAlignment": "Center" | ||
}, | ||
{ | ||
"type": "Carousel", | ||
"spacing": "Large", | ||
"pages": [ | ||
{ | ||
"type": "CarouselPage", | ||
"id": "firstPage", | ||
``` | ||
|
||
## Spacing | ||
Controls the amount of spacing between `header` and carousel page. Same enumeration value of AdaptiveCard element property `Spacing` is applied. | ||
`spacing` has no effect if either of header and spacing is missing. | ||
|
||
## HostConfig Options | ||
|
||
* Max number of `CarouselPage`s | ||
* Minimum timer restriction | ||
* Do we need host config theming? We're leaning towards leaving this for native styling via CSS. | ||
## InitialPage Property | ||
* `"initialPage" : number` | ||
* Set initial Carousel Page | ||
|
||
## Open Issues | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
# Stacked Images/Badging | ||
### Problem | ||
Customers are requesting the ability to stack images like so: | ||
![img](assets/StackedImageSet/stackedImage.png) | ||
|
||
This will enable them to show a chat as a group chat in a messenger app. | ||
#### Key Features | ||
* Stack a maximum of 2 images. The green status indicator would not be displayed. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason for displaying only 2 images? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this is client specific feature. we are going to share this spec with Team to expand the spec further. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
* The images will always be stacked diagonal as shown above. | ||
* The front overlapping image needs to have a border that is the same color as the background so that the stacking appears natural. | ||
|
||
### Alternative Considered | ||
Users can achieve a similar behavior by using background images | ||
![img](assets/StackedImageSet/currentStackedImage.png) | ||
|
||
In this example the image on the right is a regular image using `"style": "person"` and the bigger image is a circular background image with a transparent background. The issues here are that: | ||
* It is difficult to place the regular image where it needs to be. | ||
* The front image does not have a border | ||
|
||
### Proposal | ||
The review we had in the design discussion suggested we add 2 properties to [`ImageSet`](https://adaptivecards.io/explorer/ImageSet.html) for this. | ||
|
||
* `style`: "stacked" | ||
* indicates that images are displayed in a stack | ||
* `offset`: a number | ||
* indicates the magnitude of offset in pixels. This is how far apart diagonally the images are at. Images are at tangent at 0. In other word, there will be no space between the images except the border. Negative values moves images closer up to the image's diameter. Positive values move images farther and there is no limitation imposed by the spec, but will result in bad UI with extreme value. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there some way to specify size of the stacked on top of the other image or is the stacked image always a certain size? |
||
|
||
|Capability|Priority| | ||
|---|---| | ||
|Allow users to stack at least 2 images |p0 | ||
|The stacked image sets support "style": "person" so they are rounded|p0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry, I don't see the extra tab. |
||
|The stacked images stack diagonally with the front image on the left |p0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. terminate line with | |
||
|The front image has a border that matches the background of the parent container for blending purposes |p0 | ||
|The stacked images can stack from left to right |p2 |
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.
suggestion: It's relevant to notice