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

Support explicit column width in pixels #1026

Closed
6 tasks done
dclaux opened this issue Dec 4, 2017 · 12 comments
Closed
6 tasks done

Support explicit column width in pixels #1026

dclaux opened this issue Dec 4, 2017 · 12 comments

Comments

@dclaux
Copy link
Member

dclaux commented Dec 4, 2017

Implementation status

Problem

Achieving table-like layouts with Adaptive is hard because one has to use several ColumnSets on top of each other and there are only two ways to ensure that all columns in all those set have the same widths:

  • Use weights
  • Use auto and make sure the content always has the same width. That is easy enough with images, but very hard (near impossible) with text

Solution

Allow columns to have an explicit width, expressed in pixels. Essentially, make it possible in an easy and straightforward way to do what is already possible with a column containing an explicitly sized invisible image, which card authors will do to achieve the results they want.

Property Type Description
Column.width string Defines the width of the column in pixels. Width can be either a number or a string. To specify a width in pixels, set the unit px. eg: "width": "50px".

Example use:

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"version": "1.0",
	"body": [
		{
		    "type": "ColumnSet",
		    "columns": [
		        {
		            "width": "50px",
		            "items": [
		                {
		                    "type": "Image",
		                    "url": "http://3.bp.blogspot.com/-Xo0EuTNYNQg/UEI1zqGDUTI/AAAAAAAAAYE/PLYx5H4J4-k/s1600/smiley+face+super+happy.jpg",
		                    "size": "stretch"
		                }
		            ]
		        },
		        {
		            "width": "stretch",
		            "verticalContentAlignment": "center",
		            "items": [
		                {
		                    "type": "TextBlock",
		                    "text": "This card has two ColumnSets on top of each other. In each, the left column is explicitly sized to be 50 pixels wide.",
		                    "wrap": true
		                }
		            ]
		        }
		    ]
		},
		{
		    "type": "ColumnSet",
		    "columns": [
		        {
		            "width": "50px"
		        },
		        {
		            "width": "stretch",
		            "verticalContentAlignment": "center",
		            "items": [
		                {
		                    "type": "TextBlock",
		                    "text": "In this second ColumnSet, columns align perfectly even though there is nothing in the left column.",
		                    "wrap": true
		                }
		            ]
		        }
		    ]
		}
	]
}

As rendered:
image

@dclaux dclaux added this to the v1.1 milestone Dec 4, 2017
@khouzam khouzam removed this from the v1.1 milestone Mar 15, 2018
@khouzam khouzam added this to the 1804 milestone Apr 5, 2018
@khouzam khouzam added the Epic label Apr 5, 2018
@khouzam khouzam removed this from the 1804 milestone Apr 5, 2018
@khouzam
Copy link

khouzam commented Apr 6, 2018

Hey David,

Since Width is already a string couldn't we use that string with a differentiator to specify the width in pixels? It would save on the ambiguity when both width and pixelWidth are specified.

How do we handle?

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"version": "1.0",
	"body": [
		{
		    "type": "ColumnSet",
		    "columns": [
		        {
		            "pixelWidth": 50,
                            "width": 50
                        },
		        {
		            "width": "stretch"
                        }

Why not do what HTML does and have 50px mean that the units are pixels?

Thoughts?

@dclaux
Copy link
Member Author

dclaux commented Apr 6, 2018

The reason I went with pixelWidth is we also need explicit image sizes in pixels. But Image doesn't have a width nor a height property and I didn't want to overload the size property so it supports syntax line "50px*50px" which I think is horrible. A cleaner approach IMO was to introduce a width and a height property on Image, and given that the only accepted unit would be pixels I went for pixelWidth and pixelHeight. And therefore, I opted for pixelWidth for Column as well for consistency.

Overall it seems to me that introducing explicit properties for pixel sizes is cleaner. The rule would be that pixelWidth wins over width (same on Image).

Thoughts?

@khouzam
Copy link

khouzam commented Apr 9, 2018

David,

Thanks for the explanation, with the image scenario, I completely agree that overloading size and having to parse a string to extract both the width and the height would be horrible.

But pixelWidth and pixelHeight start becoming restrictive if we want to introduce new measures and units in the future.

If we keep the names generic width and height for images, we could take pixels initially and have the initial version only accept pixels with the px unit specified in the string, but keeping the possibility in the future of accepting other units or ways of specifying the width and the height without introducing more properties.

The rule would still apply that width and height take precedence over the size parameter for images.

@dclaux
Copy link
Member Author

dclaux commented Apr 9, 2018

I used the names pixelWidth and pixelHeight so as to be able to type them as number and be explicit as to what unit they were expressed in.

What you suggest is definitely an option. However it requires that we make these properties strings instead of numbers. I would prefer using actual numbers when possible, it makes payload validation and schema-based IntelliSense easier. Also note that the "width" property in Column can be expressed as a number, representing a weight. I'd say it would be a bit weird to let authors specify a weight as a number but pixels as a string.

Typically, if we used "width" and "height" as string, it would likely be so we support values like "*" for stretch, "auto" for automatic, "150px" for explicit pixels. Both "*" and "auto" are already handled via the "size" property. So if we used "width" and "height" as strings IMO it would really only be in order to maybe support additional units in the future, such as "mm", "in", "em" or whatnot, and I am really not sure we would ever need to go there.

All that said, I am open. If there is a consensus around "width" and "height" as strings, I can live with it.

@matthidinger
Copy link
Member

matthidinger commented Apr 12, 2018

My opinion for future-proofing would be either (in order of preference):

  1. Use type string with a syntax of <number><unit>. E.g., 50px or 2*
    This seems aligned with what all other UI platforms have done to my knowledge.

  2. Use type object with named properties for the value, and the unit

{
  "type": "Image",
  "width": { 
     "value": 50,
     "unit": "px"
  }
}

@dclaux
Copy link
Member Author

dclaux commented Apr 12, 2018

The question is, do we actually need to future proof this? This page lists supported units in CSS; I do not imagine we'd ever want to support any of them aside from % (e.g. our weight sizing for Column) and explicit pixels...

@dclaux
Copy link
Member Author

dclaux commented Apr 12, 2018

BTW we might want to make a quick decision here, as I believe pixelWidth/pixelHeight is already in the process of being implemented in either the Android or iOS SDK (or both) and if we go with that for image then we should do the same for Column IMO.
Edit: it's in the UWP renderer: #1373

@khouzam
Copy link

khouzam commented Apr 16, 2018

I would rather keep Width and Height and have them as strings for future proofing even if we don't make use of other units.

@dclaux
Copy link
Member Author

dclaux commented Apr 16, 2018

I can live with that. That said going with this approach on Column will introduce some inconsistency given that Column.width can be expressed as a number to represent a weight.

@jwoo-msft
Copy link
Member

I have one question regarding to relative width; i.e. width without unit (e.g. "px"). in a columnset with both explicit and relative width, how relative width should be handled? For example, column 1 has "50px" and column 2 has "5". Should we consider explicit width in determining the width for relative widths?

@dclaux
Copy link
Member Author

dclaux commented Apr 20, 2018

Any column that requests an explicit width in pixels gets what it asked for. All other columns (weight, auto or stretch) share the remaining space.

@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

5 participants