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

Updated Carousel and ImageSet spec #8220

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 85 additions & 95 deletions specs/DesignDiscussions/Carousel.md
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.
Expand All @@ -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!"
}
]
}
}
]
}
Expand All @@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll probably notice

suggestion: It's relevant to notice

* 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

missing closing ')'

Copy link
Contributor

Choose a reason for hiding this comment

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

would be a collection of CarouselPages

suggestion: is a collection of 'Carousel Page's


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
Expand All @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Expand Down
34 changes: 34 additions & 0 deletions specs/DesignDiscussions/StackedImageSet.md
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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for displaying only 2 images?
3 and more could be displayed in a slightly different fashion (similar to Teams?)

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Change for spaces

* 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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

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

extra tab

Copy link
Member Author

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.