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

[DevSpec] Underline #3277

Merged
merged 19 commits into from
Aug 17, 2020
Merged

[DevSpec] Underline #3277

merged 19 commits into from
Aug 17, 2020

Conversation

almedina-ms
Copy link
Contributor

@almedina-ms almedina-ms commented Jul 30, 2019

This is the devspec for underline

image

Microsoft Reviewers: Open in CodeFlow


### Samples
The following samples will be added to validate rendering:
- TextBlock.Underline.json file which contains TextBlock with underline
Copy link
Contributor

Choose a reason for hiding this comment

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

In the proposal review and the spec, we decided to only add underline to TextRun. Whether we add the property to TextBlock (and all other TextRun properties to TextBlock) needs further discussion.

Copy link
Member

Choose a reason for hiding this comment

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

Who is requesting this feature again and are they only requesting it for TextRun or also TextBlocks? What is the estimated cost for adding underllines to TextBlock as well? If not significant should we just include it if it makes sense and facilitates usability of the feature (so the customer doesnt have to remember which one to pick that has underline support) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The customer only needs it for underline, and we had a disagreement about whether features in TextRun should also be on TextBlock, that disagreement needs to be solved before we add this feature to TextBlock (Matt and David disagreed with adding underline to TextBlock).

specs/DevSpecs/TextBlock.Underline.md Outdated Show resolved Hide resolved
@ghost ghost removed the Needs: Author Feedback label Aug 2, 2019

### UWP & .NET WPF

TextBlock has a property called [TextDecorations](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.controls.textblock.textdecorations) which includes Underline
Copy link
Contributor

Choose a reason for hiding this comment

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

TextBlock [](start = 0, length = 9)

update actual type #Closed

@shalinijoshi19
Copy link
Member

@almedina-ms / @andrewleader, are we able to now close this PR out given the code itself has been merged?

@andrewleader
Copy link
Contributor

Seems good to merge/close, the spec for underline should also be merged into master if the feature is coded up everywhere: #3086

@ghost ghost added the no-recent-activity label Nov 26, 2019
@ghost ghost assigned dclaux Nov 26, 2019
@ghost
Copy link

ghost commented Nov 26, 2019

This Pull request has had no recent activity for the past 2 days . Please take the necessary actions (review, address feedback or commit if reviewed already) to move this along.

@almedina-ms almedina-ms changed the base branch from master to main August 13, 2020 21:03
@shalinijoshi19
Copy link
Member

@almedina-ms this now goes into spec\designDiscussions ; Let;s get this checked in.

Copy link
Contributor

@RebeccaAnne RebeccaAnne left a comment

Choose a reason for hiding this comment

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

:shipit:

@almedina-ms almedina-ms merged commit a847e06 into main Aug 17, 2020
@almedina-ms almedina-ms deleted the user/almedina-ms/DevSpecUnderline branch August 17, 2020 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants