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

Allow elements to stretch vertically to make it possible to align content to the bottom of a fixed-height card #484

Closed
6 tasks done
dclaux opened this issue Jul 17, 2017 · 25 comments

Comments

@dclaux
Copy link
Member

dclaux commented Jul 17, 2017

Implementation status

Problem

Currently, it is not really possible to align content at the bottom of a fixed-height card. Card authors have to recourse to hacks such as inserting an invisible image of a specific height to reserve some blank space. Such hacks are very fragile because they rely on knowing what the actual height of the target container is; cards authored using such hacks risk breaking if the container's height changes.

Design

Add a new height property to all element types:

Property name Type Description
height string. Allowed values are "auto" and "stretch" Defines how the element's height is computed within its parent container. When set to "auto" (default), the height of the element is what it needs. When set to "stretch", the element stretches to the available height in its parent container. When multiple elements have their height property set to "stretch", they share the available height equally.

Because we are introducing a "height" property, it makes sense to also rename the "size" property of a Column to "width"

Example

Card payload

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"height": "stretch",
	"body": [
		{
			"type": "Container",
			"items": [
				{
					"type": "TextBlock",
					"text": "This is the top container.",
					"wrap": true
				}
			]
		},
		{
		    "type": "Container",
			"spacing": "large",
			"separator": true,
		    "items": [
		        {
		            "type": "TextBlock",
		            "text": "This is the middle container. Its **height** property is **stretch** so it uses all the remaining vertical space.",
		            "wrap": true
		        }
		    ]
		    
		},
		{
			"type": "ColumnSet",
			"height": "stretch",
			"columns": [
			    {
			        "width": "auto",
			        "items": [
						{
							"type": "Image",
							"url": "http://connectorsdemo.azurewebsites.net/images/MSC12_Oscar_002.jpg",
							"size": "small",
							"style": "person"
						}
			        ]
			    },
			    {
			        "width": "stretch",
			        "spacing": "large",
			        "separator": true,
			        "items": [
	                    {
	                        "type": "TextBlock",
	                        "height": "stretch",
	                        "text": "It also works within a ColumnSet",
	                        "wrap": true
	                    },
			            {
			                "type": "TextBlock",
			                "text": "This text aligns with the bottom of the column",
	                        "wrap": true
			            }
			        ]
			    }
			]
		},
		{
		    "type": "Container",
			"spacing": "large",
			"separator": true,
			"items": [
				{
					"type": "TextBlock",
					"text": "This is the bottom container.",
					"speak": "",
					"wrap": true
				}
			]
		}
	]
}

As rendered in the Windows Live Tile container:

image

Another example

Card payload

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"height": "stretch",
	"body": [
		{
			"type": "ColumnSet",
			"columns": [
				{
					"width": 1,
					"items": [
						{
							"type": "TextBlock",
							"text": "The left column has a very, very long text which makes it very tall. Because it is tall, the right column is also very tall. The right column contains two TextBlocks; one is displaying at the top of the column, while the other one is displayed at the bottom. That is achieved by setting the first TextBlock's **height** property to **stretch**.",
							"wrap": true
						}
					]
				},
				{
					"width": 1,
					"spacing": "large",
					"separator": true,
					"items": [
						{
							"type": "TextBlock",
							"height": "stretch",
							"text": "First TextBlock displayed at the top",
							"wrap": true
						},
						{
							"type": "TextBlock",
							"text": "Second TextBlock displayed at the bottom",
							"wrap": true
						}
					]
				}
			]
		}
	]
}

As rendered

image

@dclaux dclaux changed the title Allow containers to stretch vertically to make it possible to align content to the bottom of a fixed-height card Allow elements to stretch vertically to make it possible to align content to the bottom of a fixed-height card Jul 17, 2017
@matthidinger
Copy link
Member

@Anbare this proposal would add some pretty neat flexibility for fixed-height cards. Any thoughts on this?

@andrewleader
Copy link
Contributor

People have wanted bottom-aligned content since the Windows 10 live tile days - ship it.

The proposal seems to work. Adding vertical alignment would make sense though.

@danmarshall
Copy link
Contributor

danmarshall commented Jul 17, 2017

I see the point about vertical alignment. If the textblock "It also works within a ColumnSet" was removed, then the textblock "This text aligns with the bottom of the column" would no longer align to bottom?

@andrewleader
Copy link
Contributor

Correct. Having to think about the height of the previous element in order to align something to the bottom is a bit funky. Having two text blocks with normal (auto) heights and then just setting the second one to bottom alignment would be more intuitive based on today's UI languages.

@dclaux
Copy link
Member Author

dclaux commented Jul 17, 2017

The only thing that's missing really is the ability to center something vertically. But this feature actually makes it possible as well. If you wanted to have a single TextBlock centered vertically, you could have:

  • One containr with height = stretch
  • The TextBlock
  • Another container with height = stretch

How about that? It seems to me that might be enough. I can add an example here a bit later if necessary.

@andrewleader
Copy link
Contributor

I'm arguing that it would be a lot more straightforward and intuitive to simply add a verticalAlignment property. I know it's not technically required - it's just a lot simpler.

@dclaux
Copy link
Member Author

dclaux commented Jul 17, 2017

Explicit vertical alignment would introduce the below as side effects:

  • Elements do not necessarily visually appear in the order they are declared anymore
  • Elements aligned similarly have to be rendered in separate containers, and it's the container that gets the vertical alignment treatment. That complicates renderer implementations (short of doing that two elements aligned at the bottom would appear on top of each other for instance)

The height property proposed here doesn't have these problems, although it does require that one thinks in terms of "reserving empty space" rather than vertical alignment; that said, when you think about it, it is the exact same principle as sizing columns.

@dclaux
Copy link
Member Author

dclaux commented Jul 17, 2017

Also, an important thing to not forget is that the proposed height property is still necessary anyway in order to support vertical centering. So what I'm arguing at this point is that because height is necessary, and because it makes it possible to do all sorts of alignment, I think we can start with it without introducing explicit vertical alignment.

@andrewleader
Copy link
Contributor

I don't understand your second side effect, could you elaborate more?

For the first side effect, we wouldn't necessarily have to render out of order. Elements could still appear in the order that they're declared. If you assigned "Top" after you first had an item assigned "Bottom", that second item would still be displayed beneath the bottom item - we don't have to perform logic to attempt to figure out what the developer meant in this weird case.

I know you're trying to simplify things by building this feature with the minimal set of properties required. I'm arguing that it benefits the developer story by adding an extra (technically not necessary) property.

Basically - what would be the most intuitive way a developer would think to design the picture you showed. For a XAML dev, they would assign the height of the column set to stretch, and then they would assign the second text block's vertical alignment to bottom. I don't believe that making the first text block stretch height in order to achieve bottom alignment of the second text block wouldn't be intuitive to XAML devs.

@dclaux
Copy link
Member Author

dclaux commented Jul 17, 2017

My second "side effect" is about making things just work. If a card author writes this:

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"body": [
		{
			"type": "TextBlock",
			"text": "First text block"
		},
		{
			"type": "TextBlock",
			"text": "Second text block",
                        "verticalAlignment": "bottom"
		},
		{
			"type": "TextBlock",
			"text": "Third text block",
                        "verticalAlignment": "bottom"
		}
	]
}

I think it should render like this (otherwise if I want two elements at the bottom I will have to use the height property as proposed in this issue anyway):
image

Your parallel with XAML is two faced:

  • XAML does have a vertical alignment property which applies in Grid where everything is essentially positioned absolutely

    • XAML will not render the above the way I think it should be; because Grid uses absolute positioning, two elements aligned to the bottom will render on top of each other:
      image
  • Adaptive is essentially a XAML StackPanel, and the XAML StackPanel does not support vertical alignment for its elements

I am very confused by your first point about my first side effect. Let's take this card as an example:

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"body": [
		{
			"type": "TextBlock",
			"text": "First text block",
                        "verticalAlignment": "bottom"
		},
		{
			"type": "TextBlock",
			"text": "Second text block",
                        "verticalAlignment": "top"
		}
	]
}

How should it render according to what you're describing above?

@andrewleader
Copy link
Contributor

Let's just leave this up to a third voice to come in and vote, since we're not going to convince each other.

The last example would render with both text elements stacked on the bottom, since the first one sends it to the bottom, and then the second one still has to display beneath the first one since the order of the elements is still respected, so it would be displayed beneath at the "top" of the remaining bottom space.

@dclaux
Copy link
Member Author

dclaux commented Jul 17, 2017

Maybe @danmarshall wants to chime in again.

But let me still react on what you just wrote - I don't think that's realistic. Elements align within their container. You're saying the first element is sent to the bottom (so the bottom of its container, right?) then the next element is rendered below that... outside/below its own container?

@danmarshall
Copy link
Contributor

@dclaux in the first stretch example, can you give an example of what it might look like if there were 3 textblocks? Or 4 - would the spacing be divided appropriately?

Also, I would guess that in this example:

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"body": [
		{
			"type": "TextBlock",
			"text": "First text block",
                        "verticalAlignment": "bottom"
		},
		{
			"type": "TextBlock",
			"text": "Second text block",
                        "verticalAlignment": "top"
		}
	]
}

That the verticalAlignment would not be valid. So this would also make for a bad developer story IMHO.

Perhaps verticalAlignment would not be settable by a child but only on the container? That is to say, it would not be on the textblocks, it would be on the same node that specified

		{
			"type": "ColumnSet",
			"height": "stretch",
			"verticalAlignment": "bottom",

But I haven't thought this through completely.

@dclaux
Copy link
Member Author

dclaux commented Jul 18, 2017

@danmarshall first let me go back to the "bottom then top" example. We can totally make it work, that is what I was alluding to above. The logic would be:

  • Before rendering a container, the renderer groups top-aligned, center-aligned and bottom-aligned elements into 3 distinct collections
  • Each collection is rendered as an individual container (a stack of elements) in the order the elements were found in the payload
  • These individual containers are positioned at the top, center and bottom of the original container.

But the drawback is now that the first element in the payload is rendered visually below the second one. Not the end of the world though. Also, please don't forget that to support center alignment we'd still need the height property as spece'd here.

Now I assume this is what you're asking - notice how effortless is to evenly spread these text blocks:

image

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"height": "stretch",
	"body": [
		{
			"type": "Container",
			"items": [
				{
					"type": "TextBlock",
					"text": "This is the top container.",
					"wrap": true
				}
			]
		},
		{
		    "type": "Container",
			"spacing": "large",
			"separator": true,
		    "items": [
		        {
		            "type": "TextBlock",
		            "text": "This is the middle container. Its **height** property is **stretch** so it uses all the remaining vertical space.",
		            "wrap": true
		        }
		    ]
		    
		},
		{
			"type": "ColumnSet",
			"height": "stretch",
			"columns": [
			    {
			        "width": "auto",
			        "items": [
						{
							"type": "Image",
							"url": "http://connectorsdemo.azurewebsites.net/images/MSC12_Oscar_002.jpg",
							"size": "small",
							"style": "person"
						}
			        ]
			    },
			    {
			        "width": "stretch",
			        "spacing": "large",
			        "separator": true,
			        "items": [
	                    {
	                        "type": "TextBlock",
	                        "height": "stretch",
	                        "text": "It also works within a ColumnSet",
	                        "wrap": true
	                    },
	                    {
	                        "type": "TextBlock",
	                        "height": "stretch",
	                        "text": "Other text",
	                        "wrap": true
	                    },
	                    {
	                        "type": "TextBlock",
	                        "height": "stretch",
	                        "text": "And one more",
	                        "wrap": true
	                    },
			            {
			                "type": "TextBlock",
			                "text": "This text aligns with the bottom",
	                        "wrap": true
			            }
			        ]
			    }
			]
		},
		{
		    "type": "Container",
			"spacing": "large",
			"separator": true,
			"items": [
				{
					"type": "TextBlock",
					"text": "This is the bottom container.",
					"speak": "",
					"wrap": true
				}
			]
		}
	]
}

@danmarshall
Copy link
Contributor

@dclaux ok that answers my first question. Then, is there a way to have only one textblock aligned to the bottom?

@dclaux
Copy link
Member Author

dclaux commented Jul 18, 2017

Trying to close on this.

I do agree that a verticalAlignment property could be a nice addition. But given that the height property is a requirement anyway (there is no centering in a container that adjusts its height to its content, as is currently the case in Adaptive), and given that it does allow for all alignment types (even though you have to use empty "spacer" elements in some cases) I think the proposed design in this issue is the right first step, and we can revisit verticalAlignment later. Can we agree on that?

Also, truth be told, vertical alignment in HTML is very hard especially in the context of elements that are dynamically sized, like with Adaptive Cards. I have been trying to implement it since yesterday with no success. There are plenty of articles on the subject on the web, you can look at this for instance to get an idea: https://stackoverflow.com/questions/79461/vertical-alignment-of-elements-in-a-div

@dclaux
Copy link
Member Author

dclaux commented Jul 18, 2017

@danmarshall You would have to use empty containers to center or bottom align a single TextBlock, although I don't believe that would be a common thing at least when it comes to aligning to the bottom (usually, there's also something at the top...)

For example, to center:

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"height": "stretch",
	"body": [
		{
			"type": "Container",
			"items": [
				{
					"type": "TextBlock",
					"text": "This is the top container.",
					"wrap": true
				}
			]
		},
		{
		    "type": "Container",
			"spacing": "large",
			"separator": true,
		    "items": [
		        {
		            "type": "TextBlock",
		            "text": "This is the middle container. Its **height** property is **stretch** so it uses all the remaining vertical space.",
		            "wrap": true
		        }
		    ]
		    
		},
		{
			"type": "ColumnSet",
			"height": "stretch",
			"columns": [
			    {
			        "width": "auto",
			        "items": [
						{
							"type": "Image",
							"url": "http://connectorsdemo.azurewebsites.net/images/MSC12_Oscar_002.jpg",
							"size": "small",
							"style": "person"
						}
			        ]
			    },
			    {
			        "width": "stretch",
			        "spacing": "large",
			        "separator": true,
			        "items": [
			            {
			                "type": "Container",
			                "height": "stretch"
			            },
			            {
			                "type": "TextBlock",
	                        "spacing": "none",
			                "text": "This text is centered within its container.",
	                        "wrap": true
			            },
			            {
			                "type": "Container",
			                "height": "stretch"
			            }
			        ]
			    }
			]
		},
		{
		    "type": "Container",
			"spacing": "large",
			"separator": true,
			"items": [
				{
					"type": "TextBlock",
					"text": "This is the bottom container.",
					"speak": "",
					"wrap": true
				}
			]
		}
	]
}

Will render this:
image

@danmarshall
Copy link
Contributor

Yes, I would agree that the proposal is the right first step - the height property.

@andrewleader
Copy link
Contributor

Yeah agreed, we should add the height property first since it's required, and then monitor feedback about whether we should add any vertical alignment helper properties.

@dclaux
Copy link
Member Author

dclaux commented Jul 18, 2017

Awesome, thanks guys for your help. @matthidinger can we consider this approved? If so I will merge the changes into master.

@matthidinger
Copy link
Member

Sounds great to me, thanks for the discussion folks. Adding to the v1 milestone for tracking as well

@dclaux
Copy link
Member Author

dclaux commented Jul 19, 2017

The PR has been merged.

@andrewleader
Copy link
Contributor

I'll take a stab at adding this to the UWP version

@andrewleader
Copy link
Contributor

When will the online HTML visualizer on the website be updated with these new changes? I got the UWP code updated to support height, but haven't switched column "size" to "width" yet, we should probably update all the samples to use "width" too?

@paulcam206
Copy link
Member

Documentation is done and in the mahiding/site1.1 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants