[WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 #716

rafael-rosa-knowcode · 2023-08-10T23:33:09Z

GitHub Issue (If applicable): closes #708

PR Type

What kind of change does this PR introduce?

Feature

What is the current behavior?

ShadowContainer content not stretching by default

What is the new behavior?

ShadowContainer content should stretch by default to allow controls inside to properly stretch also if needed

NeumorphismHollow

NeumorphismRaising

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Docs have been added/updated
Runtime Tests and/or UI Tests for the changes have been added (for bug fixes / features) (if applicable)
Contains NO breaking changes
Associated with an issue (GitHub or internal)
Commits must be following the Conventional Commits specification.

Other information

Internal Issue (If applicable):

roubachof · 2023-08-11T12:49:49Z

Careful to not look the ShadowContainer as a classic layout. It's supposed to be shadows for its child.
For example, always keep in mind that in a near future we will also have images shadows and text shadows.

agneszitte

Can you add some tests for these changes please @rafael-rosa-knowcode ?

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs

rafael-rosa-knowcode · 2023-08-14T18:44:30Z

@roubachof
In this PR we change only the change the "container" of the element (from canvas to grid).
All color and border treatments are being carried out as before.

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs

roubachof · 2023-08-15T11:42:35Z

As I was suspecting it there are 2 majors regressions here:

Shadows are now clipped
The shadow component became a real layout with its own size whereas it should depend only on the size of its child. With this PR: when you are adding shadows, the position of the child changes.

Before	After

as I was saying in teams:
We could consider the shadow container as a "ghost" component meaning that it's only a component adapting its size to its child.
Doing so, the child is the master of the positioning and sizing.
In fact the shadow container is only the "shadow(s)" of the content. So it should be totally dependent of the positioning/sizing on the element casting the shadow (its content)

roubachof · 2023-08-18T08:20:28Z

Using a

Grid 
  -> SkCanvas 
  -> Child

structure seems hacky to me

Since the grid clips the shadows, to make it work, it means the grid should have the same size as the shadows whereas it should have the same size as its child.

Maybe a possible structure could be then:

Grid
  -> Canvas
    -> SkCanvas
  -> Child

This way the grid will have the same size of its child but the shadows will not be clipped.

github-actions · 2023-08-18T15:34:22Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-18T18:51:18Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-18T19:15:18Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-21T12:36:23Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

roubachof · 2023-08-21T15:40:04Z

@rafael-rosa-knowcode Can I have a update on why you decided not to implement the suggested implementation, and went with Grid root layout.
And also which mechanism you implemented to be sure that the container will have the size and the position of the child, and not the shadows?
I saw a lot of changes so I'm just concern we'll introduce less maintainable code than the proposed solution.

rafael-rosa-knowcode · 2023-08-21T17:20:55Z

@roubachof
It was due to the time to deliver this solution within this version.
As it had been implemented when you commented on using Grid and Canvas , and I was already trying to pass the solution on android with @carldebilly help, I ended up following the solution and adjusting the test was faster.
However, the solution of using the Grid and Canvas may be the best solution. This way we will be able to maintain the positioning and adjust the child to the available space.

With regard to the space of shadows and Container, I was unable to resolve the issue of spacing exceeding the grid margin. So i add a margin on the conteiner (panel) to the shadows use.
Because of this, switching to Canvas again may be the best solution.
But due to time, I haven't been able to deploy it yet.

So I would need guidance on that.
If I should go ahead and implement the canvas inside the grid.

Xiaoy312 · 2023-08-22T17:20:36Z

@rafael-rosa-knowcode LMK if you need help with the merge conflict. I can unify the changes from both sides.

github-actions · 2023-08-22T19:38:32Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-22T21:03:04Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

roubachof

Two nested grids seemed to be used instead of one.

src/Uno.Toolkit.Skia.WinUI/Themes/Generic.xaml

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs

src/Uno.Toolkit.Skia.WinUI/Themes/Generic.xaml

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs

github-actions · 2023-08-22T21:34:32Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-22T22:21:57Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

pictos

just two small comments about the code

samples/Uno.Toolkit.Samples/Uno.Toolkit.Samples.Shared/Converters/HexToColorConverter.cs

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs

github-actions · 2023-08-23T10:44:35Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-23T10:48:59Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-23T10:52:53Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-23T11:40:15Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-23T11:47:52Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

github-actions · 2023-08-23T18:58:39Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs

jeromelaban · 2023-08-29T00:17:37Z

...kit.Samples/Uno.Toolkit.Samples.Shared/Content/Controls/ShadowContainerSamplePage.WinUI.xaml


 		<utu:Shadow x:Key="DefaultShadow"
 					BlurRadius="20"
 					OffsetX="10"
 					OffsetY="10"
 					Opacity="0.5"
 					Spread="0"
-					Color="{StaticResource UnoColor}" />
+					Color="{StaticResource UnoPink}" />


Is the pink a marker color for tests? or just debugging colors?

can you comment here to confirm? cause i think this is just for debugging, and shouldnt be here

jeromelaban · 2023-08-29T00:19:02Z

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.Properties.cs

 	}

 	private void OnShadowSizeChanged()
 	{
 		if (_currentContent != null && _currentContent.ActualWidth > 0 && _currentContent.ActualHeight > 0)
 		{
-			UpdateCanvasSize(_currentContent.ActualWidth, _currentContent.ActualHeight, Shadows);
+			InvalidateFromShadowPropertyChange();
+			this.GetDispatcherCompat().Schedule(() => UpdateCanvasSize(Shadows));


Won't this cause two schedules? One from the UpdateShadows call, and one here?

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs

Co-Authored-By: Pedro Jesus <pedrojesus.cefet@gmail.com> Co-Authored-By: Agnès ZITTE <16295702+agneszitte@users.noreply.github.com>

github-actions · 2023-08-29T21:03:03Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

jeromelaban · 2023-08-29T21:06:44Z

/azp run

azure-pipelines · 2023-08-29T21:06:55Z

Azure Pipelines successfully started running 1 pipeline(s).

agneszitte · 2023-08-29T23:29:09Z

@Mergifyio backport legacy/3x release/stable/3.1

mergify · 2023-08-29T23:29:12Z

backport legacy/3x release/stable/3.1

✅ Backports have been created

#780 [WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 (backport #716) has been created for branch legacy/3x
#781 [WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 (backport #716) has been created for branch release/stable/3.1

github-actions · 2023-08-29T23:38:37Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

rafael-rosa-knowcode · 2023-08-29T23:40:32Z

Restored

Full

github-actions · 2023-08-30T02:01:25Z

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-716.eastus2.azurestaticapps.net

Xiaoy312

looks about right, minus the code alignment
but we will take a blind eye that for now

[WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 (backport #716)

…1/pr-716 [WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 (backport #716)

rafael-rosa-knowcode force-pushed the dev/agzi/ShadowContainerStretchContentIssue branch from ea73f9d to c69a44a Compare August 11, 2023 11:16

agneszitte reviewed Aug 11, 2023

View reviewed changes

dr1rrb reviewed Aug 11, 2023

View reviewed changes

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs Outdated Show resolved Hide resolved

carldebilly reviewed Aug 14, 2023

View reviewed changes

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs Outdated Show resolved Hide resolved

agneszitte requested review from dr1rrb, jeromelaban and Xiaoy312 August 14, 2023 19:19

rafael-rosa-knowcode force-pushed the dev/agzi/ShadowContainerStretchContentIssue branch from 4534032 to 2620370 Compare August 18, 2023 18:47

carldebilly approved these changes Aug 21, 2023

View reviewed changes

rafael-rosa-knowcode force-pushed the dev/agzi/ShadowContainerStretchContentIssue branch 2 times, most recently from 9f30290 to bcc22d2 Compare August 22, 2023 19:32

roubachof requested changes Aug 22, 2023

View reviewed changes

pictos reviewed Aug 22, 2023

View reviewed changes

pictos reviewed Aug 23, 2023

View reviewed changes

src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.cs Outdated Show resolved Hide resolved

jeromelaban reviewed Aug 29, 2023

View reviewed changes

Xiaoy312 added the control/shadows label Aug 29, 2023

rafael-rosa-knowcode force-pushed the dev/agzi/ShadowContainerStretchContentIssue branch from 25668b4 to a0cf8d7 Compare August 29, 2023 19:32

chore: NeumorphismHollow and Raising

7d42a8c

Co-Authored-By: Pedro Jesus <pedrojesus.cefet@gmail.com> Co-Authored-By: Agnès ZITTE <16295702+agneszitte@users.noreply.github.com>

rafael-rosa-knowcode force-pushed the dev/agzi/ShadowContainerStretchContentIssue branch from a0cf8d7 to 7d42a8c Compare August 29, 2023 20:55

chore: add ui test cases

18c9eee

chore: update layout UI

fd06381

chore: back validation

7fab083

Xiaoy312 approved these changes Aug 30, 2023

View reviewed changes

jeromelaban merged commit c796c77 into main Aug 30, 2023
24 checks passed

jeromelaban deleted the dev/agzi/ShadowContainerStretchContentIssue branch August 30, 2023 14:33

This was referenced Aug 30, 2023

[WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 (backport #716) #780

Merged

[WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 (backport #716) #781

Merged

jeromelaban added a commit that referenced this pull request Aug 30, 2023

Merge pull request #780 from unoplatform/mergify/bp/legacy/3x/pr-716

80de980

[WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 (backport #716)

jeromelaban added a commit that referenced this pull request Aug 30, 2023

Merge pull request #781 from unoplatform/mergify/bp/release/stable/3.…

e6ec431

…1/pr-716 [WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 (backport #716)

roubachof mentioned this pull request Sep 13, 2023

[ShadowContainer] unexpected layout behaviors #677

Open

[WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 #716

[WinUI/ShadowContainer] ShadowContainer content not stretching by default #708 #716

Conversation

rafael-rosa-knowcode commented Aug 10, 2023 • edited Loading

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Other information

roubachof commented Aug 11, 2023

agneszitte left a comment

Choose a reason for hiding this comment

rafael-rosa-knowcode commented Aug 14, 2023

roubachof commented Aug 15, 2023 • edited Loading

roubachof commented Aug 18, 2023 • edited Loading

github-actions bot commented Aug 18, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 18, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 18, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 21, 2023 • edited by unfurl-links bot Loading

roubachof commented Aug 21, 2023

rafael-rosa-knowcode commented Aug 21, 2023

Xiaoy312 commented Aug 22, 2023

github-actions bot commented Aug 22, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 22, 2023 • edited by unfurl-links bot Loading

roubachof left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 22, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 22, 2023 • edited by unfurl-links bot Loading

pictos left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 23, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 23, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 23, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 23, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 23, 2023 • edited by unfurl-links bot Loading

github-actions bot commented Aug 23, 2023 • edited by unfurl-links bot Loading

jeromelaban Aug 29, 2023

Choose a reason for hiding this comment

Xiaoy312 Aug 30, 2023

Choose a reason for hiding this comment

jeromelaban Aug 29, 2023

Choose a reason for hiding this comment

github-actions bot commented Aug 29, 2023 • edited by unfurl-links bot Loading

jeromelaban commented Aug 29, 2023

azure-pipelines bot commented Aug 29, 2023

agneszitte commented Aug 29, 2023

mergify bot commented Aug 29, 2023 • edited Loading

✅ Backports have been created

github-actions bot commented Aug 29, 2023 • edited by unfurl-links bot Loading

rafael-rosa-knowcode commented Aug 29, 2023

github-actions bot commented Aug 30, 2023 • edited by unfurl-links bot Loading

Xiaoy312 left a comment

Choose a reason for hiding this comment

rafael-rosa-knowcode commented Aug 10, 2023 •

edited

Loading

roubachof commented Aug 15, 2023 •

edited

Loading

roubachof commented Aug 18, 2023 •

edited

Loading

github-actions bot commented Aug 18, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 18, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 18, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 21, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 22, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 22, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 22, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 22, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 23, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 23, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 23, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 23, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 23, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 23, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 29, 2023 •

edited by unfurl-links bot

Loading

mergify bot commented Aug 29, 2023 •

edited

Loading

github-actions bot commented Aug 29, 2023 •

edited by unfurl-links bot

Loading

github-actions bot commented Aug 30, 2023 •

edited by unfurl-links bot

Loading