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

fontType property on TextBlock #1078

Closed
6 tasks done
tongyliu opened this issue Jan 16, 2018 · 18 comments
Closed
6 tasks done

fontType property on TextBlock #1078

tongyliu opened this issue Jan 16, 2018 · 18 comments

Comments

@tongyliu
Copy link
Contributor

tongyliu commented Jan 16, 2018

Release Renderer status Tasks status
1.2 ✔️ .NET (#1967)
✔️ Android (#1964)
✔️ iOS (#1965)
✔️ TS (#2430)
✔️ UWP (#1963)
✔️ Shared (#1962)

Solves requests

  • Monospace blocks of text (Request: Monospace blocks of text #1982)
    • OPEN ISSUE: Would we need author ability to disable markdown so that spaces are preserved? Replacing them with   would be a huge amount of bloat

Solution

Add an additional fontType property to TextBlocks, with configurable fonts for each possible fontType value in host config. Monospace blocks of text can be achieved by using a container with emphasis style.

image

Schema

New property on TextBlock

Property Type Required Description
fontType FontType false Changes the font style

FontType

Value Description
default The default font type
monospace The monospace font type

Markup:

{
    "type": "TextBlock",
    "size": "large",
    "weight": "bolder",
    "text": "14,411 ft",
    "fontType": "monospace"
}

Why "fontType"?

  • Didn't choose just style since we might want to use style for something else
  • Didn't choose textStyle since can imagine this font style being useful in other places, like Actions, where the text property is called title rather than text, so font is applicable to both.
  • Didn't choose fontStyle since in HTML/UWP/Android, fontStyle (or textStyle) is used for italic/bold, not font families
  • Didn't choose fontFamily for reasons specified in a comment below
  • Didn't choose font since font is supposed to be a collection of fontFamily + size + weights

Host Config

Add new fontTypes property, which allows defining the font type for each type. Inside each, can set the fontFamily, and also the font sizes, since different fonts may appear larger or smaller. Can set font weights for same reason.

If some properties on a style aren't specified, inherits the properties from the default style.

Deprecate existing fontFamily, fontSizes, and fontWeights properties in exchange for the new fontTypes.

If a host didn't provide the default style but provided the old deprecated properties, those old deprecated properties should be utilized as the default style (and those should be inherited to other styles that also aren't specified).

{
    "fontTypes": {
        "default": {
            "fontFamily": "Arial, sans-serif",
            "fontSizes": {
                "small": 12,
                "default": 14,
                "medium": 17,
                "large": 21,
                "extraLarge": 26
            },
            "fontWeights": {
                "lighter": 200,
                "default": 400,
                "bolder": 600
            }
        },
        "monospace": {
            "fontFamily": "Consolas, serif",
            "fontSizes": {
                "small": 12,
                "default": 14,
                "medium": 17,
                "large": 21,
                "extraLarge": 26
            },
            "fontWeights": {
                "lighter": 200,
                "default": 400,
                "bolder": 600
            }
        }
    }
}

Down-level impact

Low. Text appears and displays pretty well. It simply doesn't have the correct font family.

Host burden

Medium. Changes in structure of host config.

Renderer Requirements

  1. A renderer must use correct inheritance of host config unspecified values
  2. For a given fontType, if the fontTypes.[specifiedStyle].[property] value exists, use its value
  3. Otherwise, if wasn't default, see if the default has a value for that
  4. Otherwise, see if the deprecated property has a value for that
  5. Otherwise, use initial default value
  6. A renderer should support displaying the correct fontTypes

Auto-generated task status

  • Shared
  • .NET
  • Android
  • iOS
  • TS
  • UWP
@andrewleader
Copy link
Contributor

Final spec needed. Also, hosts should be able to define different font sizes for each font in host config, since Consolas 11pt looks bigger than Segoe UI 11pt font.

@dclaux
Copy link
Member

dclaux commented Sep 14, 2018

Looks good for me overall. Only thing is I'm no fan of the term "display" - whether you use the default, display or monospace font styles, they are all for "display". I suggest we go for something like "alternate" or similar.

@andrewleader
Copy link
Contributor

Agreed - maybe "statistic"? Since that's what it's typically used for?

@mdtauk
Copy link

mdtauk commented Sep 14, 2018

Display is an existing term in type design, alongside "text" fonts. The opposite to Monospaced is Proportional fonts

@khouzam khouzam self-assigned this Sep 27, 2018
@andrewleader andrewleader changed the title Add support for multiple fonts on a single card Spec: Add support for multiple fonts on a single card Oct 1, 2018
@andrewleader andrewleader changed the title Spec: Add support for multiple fonts on a single card Spec: fontStyle property on TextBlock Oct 1, 2018
@carlos-zamora
Copy link
Member

Do we want a warning(s) in some cases where inheritance goes into action. So far I've thought about the following cases and results:

  1. If hostConfig defines any of the deprecated properties
    --> Warning: [property] has been deprecated. Use AdaptiveHostConfig.FontStyles.Default.[property] instead.
  2. If fontStyle.default wasn't defined
    --> Warning: FontStyle.Default wasn't defined. Inheriting font style from deprecated properties.
    or
    --> Warning: FontStyle.Default wasn't defined. Using system default instead.
  3. If fontStyle.[specifiedStyle] wasn't defined
    --> Warning: FontStyle.[specifiedStyle] wasn't defined. Using FontStyle.Default instead.
  4. If the desired fontStyles.[specifiedStyle].[property] wasn't defined
    --> Warning: FontStyle.[specifiedStyle].[property] wasn't defined. Using system default instead.
    or
    --> Warning: FontStyle.[specifiedStyle].[property] wasn't defined. Using FontStyle.Default instead.

Also, do we want multiple warnings to occur from the same issue?

@andrewleader
Copy link
Contributor

Good point! I suppose avoiding multiple warnings would be nice, but not too critical. We definitely should have those warnings!

@dclaux
Copy link
Member

dclaux commented Oct 3, 2018

Are we still going with "display"? I would really prefer "monospace" to make it super clear that's what it is. Authors that will use that style need to know that the host will use a monospace font, given that type of scenarios (e.g. simple tables) this is for.

@andrewleader
Copy link
Contributor

@dclaux the font Bing wants is NOT monospace (I measured it)

capture

@mdtauk
Copy link

mdtauk commented Oct 3, 2018

That Bing example uses a Serif font while the normal text is Sans Serif As most platforms do not offer both a Sans and Serif version of their UI fonts, if you wanted to include a Monospace, Proportional Sans and a Proportional Sans Serif as font styles, then that is extra work for the Host Config in the chosen platform renderer. Georgia is available on most platforms, that is a Serif type family. Microsoft uses Consolas for its go to Monospace font.

@dclaux
Copy link
Member

dclaux commented Oct 3, 2018 via email

@andrewleader
Copy link
Contributor

@matthidinger to speak to the priority of the Bing "display" font feature request.

I like the term alternative, although it's not very semantic, alternative could actually be less prominent. What they're trying to achieve is a more prominent font. Maybe featured or something. I also suggested statistic, since this is typically used when displaying statistics.

Outlook (and other hosts) don't have to provide an alternative font. They could always have it use the normal font, just like they don't have to provide a monospace font either.

@dclaux
Copy link
Member

dclaux commented Oct 3, 2018 via email

@dclaux
Copy link
Member

dclaux commented Dec 17, 2018

Can we close on this? I still have the same concerns about the "display" font:

  • I don't think we should have it:
    • The only one example provided doesn't justify adding it
    • While if we do have it hosts can basically make "default" and "display" the same, partners/developers will think there's a bug when their card doesn't actually use two distinct fonts in some hosts (like Outlook)
  • If we do have it
    • The term "display" is meaningless to the people who author cards (developers in general). We'd need a term that clearly identifies the font style as an alternative to "default": all font styles are for display purposes after all.

About the monospace font style - it is supposed to help create simple tables or code blocks. But Markdown collapses consecutive spaces. So we have two choices:

  • No Markdown for TextBlocks that use the monospace style
  • We don't introduce the monospace font style, and instead provide another way to do a code block/simple table by, for instance, introducing a CodeBlock element

If we were to do the latter, then there would be no remaining reason to support fontStyle on TextBlock.

@andrewleader
Copy link
Contributor

@matthidinger to speak towards whether Bing needs a "display" (or whatever we choose to call it) font for 1.2. An alternative could be that Bing uses extensibility, but we did already approve the proposal to add two font styles (monospace and display). I'm happy to find a better name for it! Or if Bing doesn't need it (or can use extensibility), I'm happy to cut it!

capture

For the monospace concern, we could add the isMarkdown property. That's how I'd personally solve it. Thoughts @matthidinger?

@dclaux
Copy link
Member

dclaux commented Dec 17, 2018

I am fine with the isMarkdown property as a solution. I would however call it useMarkdown, and I would also consider adding it to all containers (including the card itself) to make it easy to disable markdown for all or part of the card in one shot.

@matthidinger looking to hearing from you about the need for a "display" font. Adding this just for Bing and for that particular scenario doesn't feel right to me.

@andrewleader
Copy link
Contributor

Consensus was to remove the display value (so we're only adding monospace). Updated the spec to reflect that.

@andrewleader
Copy link
Contributor

Spec changes from meeting today with Andrew, Becky, David, and Mike

DECISION: Rename to fontType, since it refers to picking a collection of a set of values.

Card authors send...

{
    "type": "TextBlock",
    "text": "Text to display",
    "weight": "bold",
    "size": "large",
    "fontType": "default | monospace", // New property
}

Hosts in HostConfig specify

{
    "fontTypes": {
        "default": {
            "fontFamily": "Segoe UI, Tahoma, Geneva, Verdana, sans-serif",
            "fontSizes": {
                "small": 12,
                "default": 14,
                "medium": 17,
                "large": 21,
                "extraLarge": 26
            },
            "fontWeights": {
                "lighter": 200,
                "default": 400,
                "bolder": 600
            }
        },
        "monospace": {
            "fontFamily": "Consolas",
            "fontSizes": {
                "small": 11,
                "default": 13,
                "medium": 16,
                "large": 19,
                "extraLarge": 24
            },
            "fontWeights": {
                "lighter": 200,
                "default": 400,
                "bolder": 600
            }
        }
    }
}

For record purposes, other options we considered...

Call it fontFamily

Card authors send...

{
    "type": "TextBlock",
    "text": "Text to display",
    "weight": "bold",
    "size": "large",
    "fontFamily": "default | monospace", // New property
}

Hosts in HostConfig specify

{
    "fontFamilies": {
        "default": {
            "fontFamily": "Segoe UI, Tahoma, Geneva, Verdana, sans-serif",
            "fontSizes": {
                "small": 12,
                ...
            },
            "fontWeights": {
                ...
            }
        },
        "monospace": {
            ...
        }
    }
}

Concerns:

  • David and Becky feel that the fact that fontFamilies has a property in it called fontFamily plus the other properties is odd

Call it fontFamily for authors, and fonts for hosts

Card authors send...

{
    "type": "TextBlock",
    "text": "Text to display",
    "weight": "bold",
    "size": "large",
    "fontFamily": "default | monospace", // New property
}

Hosts in HostConfig specify

{
    "fonts": {
        "default": {
            "fontFamily": "Segoe UI, Tahoma, Geneva, Verdana, sans-serif",
            "fontSizes": {
                "small": 12,
                ...
            },
            "fontWeights": {
                ...
            }
        },
        "monospace": {
            ...
        }
    }
}

Concerns:

  • David and Becky feel that what the property is called in the card must correlate to what the property is called in the host config

Call it fontStyle

Card authors send...

{
    "type": "TextBlock",
    "text": "Text to display",
    "weight": "bold",
    "size": "large",
    "fontStyle": "default | monospace", // New property
}

Hosts in HostConfig specify

{
    "fontStyles": {
        "default": {
            "fontFamily": "Segoe UI, Tahoma, Geneva, Verdana, sans-serif",
            "fontSizes": {
                "small": 12,
                ...
            },
            "fontWeights": {
                ...
            }
        },
        "monospace": {
            ...
        }
    }
}

Concerns:

  • Andrew and Mike feel concerned with overloading terminology of HTML/XAML/Android. In HTML and XAML, fontStyle is used solely for "italic | oblique". Android calls textStyle="normal|bold|italic", so similarly the style is referring to modifications to the font family.

@dclaux
Copy link
Member

dclaux commented May 15, 2019

Implemented here: #2905

@andrewleader andrewleader changed the title fontStyle property on TextBlock fontType property on TextBlock May 15, 2019
paulcam206 added a commit that referenced this issue May 24, 2019
andrewleader added a commit that referenced this issue Jun 7, 2019
* Create Image.md

* Update Image.md

* Update Image.md

* More specs

* Media element

* Trying to get auto generate working

* Closer!

* [Schema] Document data URI

Feature spec #628

* [Schema] Introduce inlineAction

Original spec #147

* Specs auto-generated!

* [Schema] Add ActionSet

Spec #877

* Include marked-schema locally

* Improve formatting of markdown table

* [Schema] Update version description

Make description for version attribute a little more clear about being required for toplevel cards.

Fixes #2958

* [Schema] Add fontType to TextBlock

Spec #1078

* [Schema] Add wrap to ChoiceSet and Toggle

Spec #1887

* Move spec generation to separate module

* Standalone spec generator script

* Auto-update specs on save

* Add some readme's

* Spec updating readme

* Point people to the readme

* 1.2 features

* Move Adaptive Card rendering into the actual spec file

* Started adding action specs

* Generate host config

* More action info

* More ActionSet details

* Mock renderer statuses

* Finished with actions

* Columns

* Start adding backgroundImage

* Started working on schema-with-types

* Testing infrastructure

* Add URI support

* Add required support

* Refactor to class

* Add type references

* Add extending classes

* Add inehritance with referencing base class

* Support multiple types for single property

* Add arrays

* Add tests for arrays of base types

* Add dictionary support

* Generate typed schema schema from typed schema itself

* Add typed schema schema

* Add booleans

* Disallow additional properties

* Add extends and schema to json schema

* Started updating some of the schema

* Support multiple top-level types and other fixes

* Fix not being able to add properties to extended classes

* Add ability to change property name of type property

* Support having a default type that doesn't need type specified

* New classType schema

* Add ContainerStyle enum

* Add VerticalContentAlignment

* Support default and required in schema

* Support any object type

* Update schemas

* Add nullable

* Add any arrays

* Update schemas

* Add shorthands

* Start writing more schema

* Allow type to not be specified at top level

* Infer type names from file name

* Add more schema

* Add marker interfaces

* More elements

* Support recursive directories

* Rename to typed-schema

* Report more useful errors

* Start generating Adaptive Card schema

* Support enums

* Schema starting to work!

* Fix incorrect container property

* Fix extended classes not allowing extended properties

* Rename to src folder

* Adding columns

* Closer to multiple tiers of extending

* Support multiple inheritance

* Throw errors on unknown types

* Rename to BlockElement

* Add FactSet

* Add ImageSet

* Add Input.Text

* Added all inputs

* Add typed classes for the parser

* Create overall Schema type object

* Support loading schema from folder

* Almost got spec generation working with new format!

* Add ActionSet

* Markdown generation is getting there!

* Markdown generation almost done

* Generate enum specs

* Add enum value description support

* Add shorthand property to classes

* Update schema to support shorthand

* Update schema file

* Include lib

* Updated schema

* Add RichTextBlock

* Re-organize

* Change image uris to strings for relative urls

* Add Style property to Card

* Add Action.ToggleVisibility

* Add style to actions

* Remove duplicative BlockElement properties

* Add fallback

* Update wrap property on inputs

* Generated

* Update Input.Text selectAction

* Generate

* Add some versions

* Fix blank allOf

* Added a test that tests our samples

* Add horizontalAlignment to richtextblock

* Add minHeight

* Add minHeight to AdaptiveCard

* Add isVisible

* Update package-lock.json

* Add verticalContentAlignment to Adaptive Card

* Add height to container and columnset

* Add height to Column

* Add height to all elements

* Support overriding properties

* Fix image height property

* Fix overriding of inherited properties

* Update adaptive-card-new.json

* Make Column.items not required

* All samples valid

* Use uri-reference

* Support allowAdditionalProperties

* Allow custom enums

* card.style version = 1.2

* Include build-model

* Correctly display version number in markdown

* Support shorthands in markdown and fix background image version

* Remove generate-specs from website

* Generating site from new schema partially working

* Site generating properties

* Fix schema literals in website properties

* Make inherited detailed properties appear, and fix examples for properties

* Make type appear correctly on markdown tables

* Update toc

* Add new elements

* Re-order version property

* Show type type in type

* Support displaying enum value versions

* Style the default value correctly

* Indicate that type on inlines is required

* Support inline shorthand

* Improve image size documentation

* Fix inlineAction description

* Surface required properties at top of table

* Remove local marked-schema

* Rename BlockElement to Element

* Dispaly uri-reference as uri

* Rename fonttype sample

* Add expense report example

* Update schema file in sample

* Update ExpenseReport to use Submit

* Support case insensitive enums

* Remove generated adaptive card schema

* Make release build schema

* Remove compiled typed-script schema

* Add instruction for generating Adaptive schema

* Remove spec-generator

* Update samples schema test to use correct payload

* Remove specs

* Support multiple schema versions

* UWP test updates for renamed FontType test

* Typo in UWP test app

* Update UWP tests after sample payloads changed

* Add FontType expected tests

* Add readme for schema

* Update Visualizer to reference new schema file and all samples

* Fix iOS referencing old FontTypes payload
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

9 participants