Skip to content

Add a Vertical Text Block to the Windows Community Toolkit #2544

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

Closed
wants to merge 46 commits into from

Conversation

darthwing-duck
Copy link

Issue: #2441

PR Type

What kind of change does this PR introduce?

  • Feature
    This implements a TextBlock like control which can render vertical text. Basically it just uses DirectWrite to render an Image which is then shown using a XAML Image object and SurfaceImageSource. The control itself supports High Contrast like XAML by overriding the foreground + background of the text block when High Contrast is turned on. Narrator reads out the "text + Image" which while accurate, isn't exactly what I want. That being said since text blocks tend to be in something else, it might be ok.

  • Build or CI related changes
    This required a fix from Nikola to support the Generated Files folder.

  • Documentation content changes
    This adds documentation for the new control (DirectWriteTextBlock) in English. There are plans to translate to Japanese if the initial changes can go through.

  • Sample app changes
    There is a new page in the Sample App for the new control which can be played with.

What is the current behavior?

There is no control which supports rendering text vertically in XAML and making sure all of the individual locale behaviors are held in vertical text (ja-JP or Traditional Mongolian) is only really supported with Direct Write.

What is the new behavior?

A Control which renders text using DirectWrite has been introduced.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Docs have been added/updated which fit documentation template. (for bug fixes / features)
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

This currently doesn't contain a designer project. I'm hoping to get this checked in though if that's required, I can resubmit later.

@dnfclas
Copy link

dnfclas commented Oct 10, 2018

CLA assistant check
All CLA requirements met.

Copy link
Contributor

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

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

Added few comments. Also need to integrate project with build script to generate nugets

@@ -0,0 +1,53 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

We are moving the documentation to https://github.com/MicrosoftDocs/WindowsCommunityToolkitDocs. Please open a docs PR there and remove docs from this PR

<MinimumVisualStudioVersion>14.0</MinimumVisualStudioVersion>
<AppContainerApplication>true</AppContainerApplication>
<ApplicationType>Windows Store</ApplicationType>
<WindowsTargetPlatformVersion>10.0.17134.0</WindowsTargetPlatformVersion>
Copy link
Contributor

Choose a reason for hiding this comment

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

target version should be 17763

nmetulev and others added 6 commits October 10, 2018 20:38
* Move font loader into DirectWriteTextBlock folder as it's just there to map UWP into DirectWrite anyway. Rename it.
* Fix documentation bugs
* Fix comment at top of files.
* Make TextOrientation to be ReadingDirection and map it directly into DirectWrite
* Make some more custom enums (having a lot of issues getting C# to see these in the .bind file).
* Add retry logic to rebuild D3D device which is required sometimes.
@darthwing-duck
Copy link
Author

image

@michael-hawker
Copy link
Member

@azchohfi what behavior do you see? Maybe this is a HW issue with a specific device or OS specific (I tested on 17763 on a Surface Laptop 2)?

@juma-msft looks like there's a conflict in samples.json now too, can you update? Thanks.

Copy link
Contributor

@azchohfi azchohfi left a comment

Choose a reason for hiding this comment

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

Ok, I've found the issue. If your display scale is 100%, everything works as expected, but if you change to 150%, for example, it will render outside the layout of the control. Not sure if the layout process is not considering the scaling factor or if your DirectDraw code is not using it. It's one or the other, but this a merge blocker. And still, we need the docs.

@michael-hawker michael-hawker removed this from the 5.1 milestone Feb 7, 2019
Jun Ma (WINDOWS) and others added 3 commits February 9, 2019 17:45
… when DPI is set to 1.25 or a non round DPI.
Pull everything into my fork from windows tool kit master.
@darthwing-duck
Copy link
Author

Yeah I figured out the scaling issue tonight, it was a problem with where the scale was being applied and fixed it.

@darthwing-duck
Copy link
Author

I also just fixed the conflict in the samples.json

@darthwing-duck
Copy link
Author

hold on from looking at the PR, the merge with samples.json didn't work correctly. Correcting that now.

@mdtauk
Copy link

mdtauk commented Feb 9, 2019

Does the control only render horizontal text rotated?

The CSS writing-mode property supports horizontal and vertical text formatting, so Japanese and Chinese characters will be laid out vertically one character above the other, but the glyphs do not rotate.

https://www.w3.org/International/articles/vertical-text/

image

@darthwing-duck
Copy link
Author

No, this control basically does what the dwrite did and what css does. Basically it supports the punctuation and quotation justification for vertical text in Japanese which just rotating the text won't do. For latin languages, it more or less just looks like a rotation.

@michael-hawker michael-hawker added this to the 6.0 milestone Mar 5, 2019
@michael-hawker
Copy link
Member

@juma-msft getting back to this for 6.0 now. Looks like another conflict. To confirm this is still C++/CX not C++/WinRT?

@darthwing-duck
Copy link
Author

This is still C++/CX.

@michael-hawker
Copy link
Member

Thanks @juma-msft would it be hard for us to convert to C++/WinRT? I know there was some initial issues with the tooling, but hopefully those would be fixed now?

We have some more C++ stuff coming in the future for us to share the new namespace with, but that's going to be C++/WinRT code, so it'd be nice if we could stick to one flavor. Any thoughts?

@michael-hawker michael-hawker removed this from the 6.0 milestone Oct 24, 2019
@michael-hawker
Copy link
Member

@juma-msft, @azchohfi and I have discussed trying to switch C++ code in the Toolkit to C# as that's where the majority of our community/contributors. It makes it easier for us to manage as well as a whole and simplifies our pipeline.

Do you still have interest in this control? Is this something we should talk about contributing to WinUI now that they work in the open source space (either 2.x or 3.0 (in the future))?

FYI @ranjeshj - would this make sense as an extension/alternate mode for the built-in TextBlock instead? Have you discussed this type of formatting options for applications in general? What are your thoughts in general here?

Thoughts?

@ranjeshj
Copy link
Contributor

@codendone as FYI

@michael-hawker
Copy link
Member

At this point, we've decided to try and keep the WCT as C# only, see #3427.

@juma-msft I think this is still an important topic though to discuss with the WinUI team. We should open an issue on their repo and continue the discussion there for the future?

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

Successfully merging this pull request may close these issues.

10 participants