fix(shapes): [iOSmacOS] Fix shapes rendering issues #2997
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dr1rrb
added
platform/macos 🍏
|
|
6 tasks
6 tasks
src/SamplesApp/SamplesApp.UITests/Windows_UI_Xaml_Shapes/Basics_Shapes_Tests.cs
Outdated
Show resolved
Hide resolved
src/SamplesApp/SamplesApp.UITests/Windows_UI_Xaml_Shapes/Basics_Shapes_Tests.cs
Outdated
Show resolved
Hide resolved
src/SamplesApp/SamplesApp.UITests/Windows_UI_Xaml_Shapes/Basics_Shapes_Tests.cs
Outdated
Show resolved
Hide resolved
src/SamplesApp/SamplesApp.UITests/Windows_UI_Xaml_Shapes/Basics_Shapes_Tests.cs
Outdated
Show resolved
Hide resolved
The CommandBar should not constraints its children, doing so, Rectangles in AppBarButton won't be stretch along invalid size, but only insteed will only be stretched along the final size (Arrange)
|
The build 16514 found UI Test snapshots differences.
|
|
The build 16514 found UI Test snapshots differences.
|
|
The build 16514 found UI Test snapshots differences.
|
| { | ||
| var size = new Size(userSize.width, userSize.height); | ||
| var minSize = new Size(userSize.min.width, userSize.min.height);; | ||
| var maxSize = new Size(userSize.max.width, userSize.max.height); ; |
There was a problem hiding this comment.
Codacy found an issue: Remove this empty statement.
| userSize) | ||
| { | ||
| var size = new Size(userSize.width, userSize.height); | ||
| var minSize = new Size(userSize.min.width, userSize.min.height);; |
There was a problem hiding this comment.
Codacy found an issue: Remove this empty statement.
|
|
||
| // By default we align if UniformToFill, EXCEPT if the the userSize (or max, lowered by min) is lower than the finalSize | ||
| // For reference, it's almost equivalent to: | ||
| // var horizontally = IsNaN(userSize.Width) || (!IsInfinity(userSize.Width) && userSize.Width > finalSize.Width) || userMinSize.Width > 0; |
There was a problem hiding this comment.
Codacy found an issue: Remove this commented out code.
| var tcs = new TaskCompletionSource<object>(); | ||
|
|
||
| using (var timeout = Debugger.IsAttached ? default : new CancellationTokenSource(TimeSpan.FromMilliseconds(1500))) | ||
| using (var reg = Debugger.IsAttached ? default : timeout.Token.Register(() => tcs.TrySetCanceled())) |
There was a problem hiding this comment.
Codacy found an issue: 'timeout' is null on at least one execution path.
| } | ||
|
|
||
| public PointCollection(IEnumerable<Point> coordinates) | ||
| { | ||
| _coords = coordinates.ToList(); | ||
| _points = coordinates.ToList(); |
There was a problem hiding this comment.
Codacy found an issue: Refactor this method to add validation of parameter 'coordinates' before using it.
| TestResult = ""; | ||
|
|
||
| var tests = testNames.Split(new [] {';'}, StringSplitOptions.RemoveEmptyEntries); | ||
| var id = Guid.NewGuid().ToString("N"); |
There was a problem hiding this comment.
Codacy found an issue: Use the overload that takes a 'CultureInfo' or 'IFormatProvider' parameter.
| renderingArea.Height = renderingArea.Width = 0; | ||
| break; | ||
|
|
||
| default: |
There was a problem hiding this comment.
Codacy found an issue: Move this 'default:' case to the beginning or end of this 'switch' statement.
| grid.ColumnDefinitions.AddRange(horizontalAlignments.Select(_ => new ColumnDefinition {Width = new GridLength(itemDimensions.Width)})); | ||
| grid.RowDefinitions.AddRange(verticalAlignments.Select(_ => new RowDefinition {Height = new GridLength(itemDimensions.Height)})); | ||
|
|
||
| for (var x = 0; x < horizontalAlignments.Length; x++) |
There was a problem hiding this comment.
Codacy found an issue: Add curly braces around the nested statement(s) in this 'for' block.
| var min = Math.Min(userSize.min.width, userSize.min.height); | ||
| minSizeForScale = new Size(0, min); | ||
| } | ||
| else if (userSize.min.hasWidth && this is Path) |
There was a problem hiding this comment.
| { | ||
| TestResult = ""; | ||
|
|
||
| var tests = testNames.Split(new [] {';'}, StringSplitOptions.RemoveEmptyEntries); |
There was a problem hiding this comment.
Codacy found an issue: Refactor this method to add validation of parameter 'testNames' before using it.
|
The build 16534 found UI Test snapshots differences.
|
|
The build 16534 found UI Test snapshots differences.
|
|
The build 16534 found UI Test snapshots differences.
|
10 tasks
carldebilly
added a commit
that referenced
this pull request
Oct 1, 2020
…on iOS This bug has been introduced by PR #2997
4 tasks
carldebilly
added a commit
that referenced
this pull request
Oct 14, 2020
…on iOS This bug has been introduced by PR #2997
This was referenced Dec 4, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
GitHub Issue: private see below
Bugfix / refactor
The way that shapes was rendered on iOS has been fully refactored to properly follow the measure/arrange phases of WinUI/Uno.
What is the current behavior?
Some shapes are not rendered properly on iOS, and sometimes they are no visible at all (cf. issue linked below).
What is the new behavior?
All shapes behaves like WinUI
PR Checklist
Screenshots Compare Test Runresults.Contains NO breaking changesSome API have been aligned to match the WinUI contract