Split out Controls.Primitives package. #3660

Rosuavio · 2021-01-06T21:23:42Z

Step towards #3594

Split out AdaptiveGridView, DockPanel, StaggeredPanel, SwitchPresenter, UniformGrid, and WrapPanel to a new project Microsoft.Toolkit.Uwp.UI.Controls.Primitives.

PR Type

What kind of change does this PR introduce?

Feature

What is the current behavior?

All of our controls are in a singular Microsoft.Toolkit.Uwp.UI.Controls package. For consumers to get anyone package they have to pull in the whole package with all the controls and all there dependencies.

What is the new behavior?

AdaptiveGridView, DockPanel, StaggeredPanel, SwitchPresenter, UniformGrid, and WrapPanel are in Microsoft.Toolkit.Uwp.UI.Controls.Primitives.

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)
- Icon has been created (if new sample) following the Thumbnail Style Guide and templates
New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
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

ghost · 2021-01-06T21:23:45Z

Thanks RosarioPulella 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 🙌

Rosuavio · 2021-01-07T20:16:14Z

@michael-hawker Do you think we need to split out the the Controls.Design project also?

Rosuavio · 2021-01-08T17:16:57Z

For the Control.Primitives package, it seems we already have some classes in that namespace that we were not planning to move to that package.

Public classes...

ColorPickerSlider
DataGridCellsPresenter
DataGridColumnHeader
DataGridColumnHeadersPresenter
DataGridDetailsPresenter
DataGridFrozenGrid
DataGridRowHeader
DataGridRowsPresenter

And an internal class,
DataGridColumnHeaderInteractionInfo

michael-hawker · 2021-01-08T18:04:11Z

@michael-hawker Do you think we need to split out the the Controls.Design project also?

I've been wondering about this. I would assume we may need to. @azchohfi do you know how these get bundled? They're just included in the package somehow right?

I don't know if it'd be weird to keep the aggregated one and only have Design support if you have the full package? As soon as you'd split into the smaller packages nothing would work in VS, right?

michael-hawker · 2021-01-08T18:07:04Z

For the other classes already in the namespace. I think that's ok? Those are all more helpers to other controls and probably aren't going to be invoked individually?

Microsoft.Toolkit.Uwp.SampleApp/Pages/About.xaml

...osoft.Toolkit.Uwp.UI.Controls.Primitives/Microsoft.Toolkit.Uwp.UI.Controls.Primitives.csproj

….Uwp.UI.Controls.Primitives.csproj Co-authored-by: Michael Hawker MSFT (XAML Llama) <24302614+michael-hawker@users.noreply.github.com>

This reverts commit e885c10.

Rosuavio · 2021-01-08T21:52:48Z

So I split out a Controls.Primitive.DesingTools project, in b6ca5af. I tried to fallow the patterns I could find.

Microsoft.Toolkit.Uwp.UI.Controls.Primitives/VisualStudioToolsManifest.xml

Rosuavio · 2021-01-12T17:47:17Z

Microsoft.Toolkit.Uwp.UI.Controls.Primitives/Properties/AssemblyInfo.cs

+[assembly: InternalsVisibleTo("UnitTests.UWP")]
+[assembly: InternalsVisibleTo("UnitTests.XamlIslands.UWPApp")]


I don't know much about how XamlIslands works, do you think we need this for the primitives, @michael-hawker?

This is just exposing it to the XamlIslands tests, so we probably need to tweak those and the Unit Tests as well if they touch these controls?

It seems like the xmal islands test worked just fine without that line.

Remove NeutralResourcesLanguage on attribute on assembly, no resources or language specific code.

Rosuavio · 2021-01-13T19:16:29Z

If you don't mind @Nirmal4G reviewing my DesignTools related changes it would appreciated.

Nirmal4G · 2021-01-13T19:21:20Z

Sure @RosarioPulella

Nirmal4G

All good here.

Nirmal4G · 2021-01-13T19:28:31Z

...osoft.Toolkit.Uwp.UI.Controls.Primitives/Microsoft.Toolkit.Uwp.UI.Controls.Primitives.csproj

+
+	<ItemGroup>
+		<None Include="VisualStudioToolsManifest.xml" Pack="true" PackagePath="tools" />
+		<None Include="$(OutDir)\Design\$(MSBuildProjectName).Design*.dll;$(OutDir)\Design\$(MSBuildProjectName).Design*.pdb" Pack="true" PackagePath="lib\$(TargetFramework)\Design" />


Instead of Design*, we can use DesignTools as we are not relying on older system anymore. But it's okay now as I still have some cleanup to do with the Design projects.

I'll take it up as part of my next PR. so don't worry about it.

I figured we were going to make that change but I did not want to jump the gun. Thanks @Nirmal4G C:

michael-hawker

Seems to work great, thanks Rosario!

Let's merge this into the dev branch and focus on the next one! 🎉🎉🎉

ghost · 2021-01-13T22:58:11Z

Hello @michael-hawker!

Because this pull request has the auto merge :zap: 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.

michael-hawker · 2021-01-13T22:58:37Z

Thanks for helping take a look at this @Nirmal4G, much appreciate your expertise in this area!

ghost requested review from michael-hawker, azchohfi and Kyaa-dost January 6, 2021 21:23

michael-hawker mentioned this pull request Jan 6, 2021

[Follow-up] Remaining Items TODO for DesignTools Refactor #3620

Closed

7 tasks

Rosuavio added 3 commits January 8, 2021 11:38

Move some controls to Primitives.

6956c73

Update Primiative controls ns

e885c10

Apply change for platform uno compatiblity #3651

2fdcbe6

Rosuavio added help wanted Issues identified as good community contribution opportunities in progress 🚧 needs attention 👋 labels Jan 8, 2021

ghost removed the needs attention 👋 label Jan 8, 2021

michael-hawker reviewed Jan 8, 2021

View reviewed changes

Rosuavio and others added 6 commits January 8, 2021 13:50

Update Microsoft.Toolkit.Uwp.UI.Controls.Primitives/Microsoft.Toolkit…

7431e17

….Uwp.UI.Controls.Primitives.csproj Co-authored-by: Michael Hawker MSFT (XAML Llama) <24302614+michael-hawker@users.noreply.github.com>

Remove System.ValueTuple from Controls.Primitives

1e34abf

Added SwitchPresenter in description of Control.Primitives project

ac71f64

Revert "Update Primiative controls ns"

b6bc173

This reverts commit e885c10.

Add SwitchPresenter tag to Controls.Primitives

04869f4

Split out primitives design project

b6ca5af

Rosuavio and others added 4 commits January 11, 2021 13:04

Fix primitives deisgn tools ref

06a9822

Merge branch 'master' into split-controls

8fd51a1

Fix finding desing output for controls.primitives

96e1aba

Add smoke test for Controls.Primitivs

3fb9053

michael-hawker reviewed Jan 12, 2021

View reviewed changes

Microsoft.Toolkit.Uwp.UI.Controls.Primitives/VisualStudioToolsManifest.xml Show resolved Hide resolved

Rosuavio changed the base branch from master to dev/split-controls January 12, 2021 17:16

Rosuavio changed the title ~~[WIP] Split controls package~~ [WIP] Split out Controls.Primitives package. Jan 12, 2021

Rosuavio added the for-review 📖 To evaluate and validate the Issues or PR label Jan 12, 2021

Rosuavio changed the title ~~[WIP] Split out Controls.Primitives package.~~ Split out Controls.Primitives package. Jan 12, 2021

Rosuavio marked this pull request as ready for review January 12, 2021 17:22

Rosuavio removed the in progress 🚧 label Jan 12, 2021

List SwitchPresenter for DesignTools

4f83e08

Rosuavio commented Jan 12, 2021

View reviewed changes

Rosuavio added 3 commits January 12, 2021 13:07

Better wording for titles and descriptions around primitivs package

0043ecd

Remove exposing internals to xmalisland test, not needed.

3cc5a64

Remove NeutralResourcesLanguage on attribute on assembly, no resources or language specific code.

Remove unneded deps

3ffa9b5

Rosuavio requested a review from michael-hawker January 13, 2021 19:06

Nirmal4G approved these changes Jan 13, 2021

View reviewed changes

michael-hawker approved these changes Jan 13, 2021

View reviewed changes

michael-hawker added the auto merge ⚡ label Jan 13, 2021

michael-hawker merged commit fc221a7 into CommunityToolkit:dev/split-controls Jan 13, 2021

Rosuavio deleted the split-controls branch January 14, 2021 00:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split out Controls.Primitives package. #3660

Split out Controls.Primitives package. #3660

Rosuavio commented Jan 6, 2021 •

edited

Loading

ghost commented Jan 6, 2021

Rosuavio commented Jan 7, 2021

Rosuavio commented Jan 8, 2021

michael-hawker commented Jan 8, 2021

michael-hawker commented Jan 8, 2021

Rosuavio commented Jan 8, 2021

Rosuavio Jan 12, 2021

michael-hawker Jan 12, 2021

Rosuavio Jan 12, 2021

Rosuavio commented Jan 13, 2021

Nirmal4G commented Jan 13, 2021

Nirmal4G left a comment

Nirmal4G Jan 13, 2021

Rosuavio Jan 13, 2021

michael-hawker left a comment

ghost commented Jan 13, 2021

michael-hawker commented Jan 13, 2021

		[assembly: InternalsVisibleTo("UnitTests.UWP")]
		[assembly: InternalsVisibleTo("UnitTests.XamlIslands.UWPApp")]

Split out Controls.Primitives package. #3660

Split out Controls.Primitives package. #3660

Conversation

Rosuavio commented Jan 6, 2021 • edited Loading

Step towards #3594

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Other information

ghost commented Jan 6, 2021

Rosuavio commented Jan 7, 2021

Rosuavio commented Jan 8, 2021

michael-hawker commented Jan 8, 2021

michael-hawker commented Jan 8, 2021

Rosuavio commented Jan 8, 2021

Rosuavio Jan 12, 2021

Choose a reason for hiding this comment

michael-hawker Jan 12, 2021

Choose a reason for hiding this comment

Rosuavio Jan 12, 2021

Choose a reason for hiding this comment

Rosuavio commented Jan 13, 2021

Nirmal4G commented Jan 13, 2021

Nirmal4G left a comment

Choose a reason for hiding this comment

Nirmal4G Jan 13, 2021

Choose a reason for hiding this comment

Rosuavio Jan 13, 2021

Choose a reason for hiding this comment

michael-hawker left a comment

Choose a reason for hiding this comment

ghost commented Jan 13, 2021

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.

michael-hawker commented Jan 13, 2021

Rosuavio commented Jan 6, 2021 •

edited

Loading

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.