Skip to content

Split Markdown.* from Controls package. #3437

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

Merged
14 commits merged into from
Sep 10, 2020
Merged

Split Markdown.* from Controls package. #3437

14 commits merged into from
Sep 10, 2020

Conversation

azchohfi
Copy link
Contributor

Created a new Microsoft.Toolkit.Uwp.UI.Controls.Markdown package, moving the MarkdownTextBlock from the Controls package and decoupling the TextToolbar from the MarkDownFormatter.

Helps with #3145

PR Type

What kind of change does this PR introduce?

  • Refactoring

What is the current behavior?

Having a dependency on Microsoft.Toolkit.Uwp.UI.Controls pulls too many packages and dependencies.

What is the new behavior?

Microsoft.Toolkit.Uwp.UI.Controls no longer has a dependency on ColorCode.UWP, nor on the Microsoft.Toolkit.Parsers package.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • 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

If you are using MarkdownTextBlock, or the TextToolbar with Format = Format.Markdown, you will need to add a dependency to the new Microsoft.Toolkit.Uwp.UI.Controls.Markdown. The MarkdownTextBlock implementation is unchanged, but the TextToolbar no longer has a Format property. The markdown parser is now provided just as any other custom parser, and need to be added in Xaml, or code behind. For example:

<Page 
...
      xmlns:markDown="using:Microsoft.Toolkit.Uwp.UI.Controls.TextToolbarFormats.MarkDown"
      xmlns:richText="using:Microsoft.Toolkit.Uwp.UI.Controls.TextToolbarFormats.RichText"
...
>
...
<controls:TextToolbar ...>
      <controls:TextToolbar.Formatter>
        <richText:RichTextFormatter />
        <!--<markDown:MarkDownFormatter />-->
        <!--<local:CustomFormatter />-->
      </controls:TextToolbar.Formatter>
</controls:TextToolbar>
...

…ing the MarkdownTextBlock from the Controls package and decoupling the TextToolbar from the MarkDownFormatter.
@ghost
Copy link

ghost commented Aug 18, 2020

Thanks azchohfi for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost requested a review from michael-hawker August 18, 2020 01:34
@azchohfi azchohfi added this to the 7.0 milestone Aug 18, 2020
@ghost ghost requested a review from Kyaa-dost August 18, 2020 01:35
@azchohfi azchohfi requested a review from Sergio0694 August 18, 2020 17:38
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

@QuinnDamerell @paulbartrum your contribution of the MarkdownTextBlock was before my time. I just noticed we had the about.txt file here. Though it looks like you contributed directly, so that was a relicense to the MIT at that time I assume? @deltakosh?

For consistency, I'd like to move this attribution instead to our ThirdPartyNotice file:

QuinnDamerell/UniversalMarkdown (https://github.com/QuinnDamerell/UniversalMarkdown) contributed by Quinn Damerell and Paul Bartrum for the MarkdownTextBlock control, relicensed to this .NET Foundation project under the MIT license upon contribution.

FYI @clairernovotny.

@michael-hawker
Copy link
Member

@Kyaa-dost was thinking we maybe want another project board to track our more 'backlog'/'debt' items. Like we have a feature/bug board, but this is neither. So maybe we want a 'Technical' bucket for 7.0? And we can use that to track all these breaking changes and other project/repo related work?

…olbarPage.xaml.cs

Co-authored-by: Michael Hawker MSFT (XAML Llama) <[email protected]>
@ghost
Copy link

ghost commented Aug 19, 2020

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

@paulbartrum
Copy link

@QuinnDamerell @paulbartrum your contribution of the MarkdownTextBlock was before my time. I just noticed we had the about.txt file here. Though it looks like you contributed directly, so that was a relicense to the MIT at that time I assume? @deltakosh?

I contributed directly to the UniversalMarkdown project, @QuinnDamerell as the project owner can speak to licensing.

@ghost ghost removed the needs attention 👋 label Aug 20, 2020
Copy link
Contributor

@vgromfeld vgromfeld left a comment

Choose a reason for hiding this comment

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

:shipit:

@michael-hawker michael-hawker added the legal review ⚖ For issues that have questions around legal matters in attribution and such that are awaiting info. label Sep 1, 2020
@michael-hawker
Copy link
Member

Hi @paulbartrum looks like we don't have a CLA sign-off on file from you from the original PR contribution where Quinn re-assigned the code. Would you mind signing our CLA here now so that we can just clean-up the history here for the future? Thanks!

@paulbartrum
Copy link

Hi @paulbartrum looks like we don't have a CLA sign-off on file from you from the original PR contribution where Quinn re-assigned the code. Would you mind signing our CLA here now so that we can just clean-up the history here for the future? Thanks!

Sure @michael-hawker, done.

@michael-hawker
Copy link
Member

Thanks so much @paulbartrum! That's a great help for resolving this scenario!

@michael-hawker michael-hawker removed the legal review ⚖ For issues that have questions around legal matters in attribution and such that are awaiting info. label Sep 10, 2020
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Think we're all clear and good to go now! I'll follow-up on anything else later if need be, but think we should be unblocked here.

@ghost
Copy link

ghost commented Sep 10, 2020

Hello @michael-hawker!

Because this pull request has the auto merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 609430a into master Sep 10, 2020
@ghost ghost deleted the splitMarkdown branch September 10, 2020 17:02
This pull request was closed.
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.

5 participants