Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Add Rect struct #11187

Merged
merged 11 commits into from
Jul 20, 2020
Merged

Add Rect struct #11187

merged 11 commits into from
Jul 20, 2020

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Jun 24, 2020

Description of Change

Allow to create a Rect struct (X, Y, Height, Width) from XAML.

API Changes

Added:

struct Rect
{
     public double X { get; set; }

     public double Y { get; set; }

     public double Width { get; set; }

     public double Height { get; set; }
}

Platforms Affected

  • Core/XAML (all platforms)

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Launch Core Gallery, navigate to the issues and search the "Rect Test" sample. If the

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

{
[DebuggerDisplay("X={X}, Y={Y}, Width={Width}, Height={Height}")]
[TypeConverter(typeof(RectTypeConverter))]
public struct Rect
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we mark the Rectangle struct as deprecated?

Copy link

Choose a reason for hiding this comment

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

Hi @jsuarezruiz - marking Rectangle struct as deprecated while releasing this new struct is as good as renaming existing
Rectangle struct.

Introduction of new set of APIs such as Shapes - should rather come in its own namespace or it should have distinct type names that won't create any ambiguity with existing ones such as this Rectangle struct.

I sincerely request Xamarin Forms team to release any new or (especially or ambiguous) breaking API changes only in the pre1 of the new XF version. That will give community, OSS vendors and solution providers couple of cycles to find such issues before that XF version is released for GA.

This time, Graph API was introduced in XF 4.7 pre4 and within couple days XF 4.7 GA was released.

Thank you for your help and support.

Copy link
Member

Choose a reason for hiding this comment

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

@jsuarezruiz it should be Obsoleted, but as @IoTFier mentioned, it's way too late in the 4.7.0 cycle to deprecate anything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, should be something to do in 4.8.0.

Copy link

@npagare npagare Jun 26, 2020

Choose a reason for hiding this comment

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

What if there are other breaking changes in my App due to XF 4.7 ?
Shall I discover those in 4.8? Disagree with this strategy.

Please consider fixing this in XF 4.7 service release - if needed refactor for good in 4.8.
One more time - Let Shapes API be in its own namespace. That can resolve many other (if at all) issues related to types defined in Shapes that are conflicting with types from Xamarin.Form.Core .

Copy link
Member

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

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

I would retarget this change, potentially breaking, to 4.8.0

{
[DebuggerDisplay("X={X}, Y={Y}, Width={Width}, Height={Height}")]
[TypeConverter(typeof(RectTypeConverter))]
public struct Rect
Copy link
Member

Choose a reason for hiding this comment

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

@jsuarezruiz it should be Obsoleted, but as @IoTFier mentioned, it's way too late in the 4.7.0 cycle to deprecate anything

@npagare
Copy link

npagare commented Jun 25, 2020

I would retarget this change, potentially breaking, to 4.8.0

@StephaneDelcroix - does this mean, I should hold off from upgrading my app to XF 4.7 ?
I would rather have an opportunity to upgrade my App to XF 4.7; and find out if there are any other breaking changes that should be factored in 4.8.

Again - the name "Rect" is too odd and ambiguous. Any thoughts on having a separate name space for Shapes that we can always refer in the XAML ?

@samhouts samhouts changed the base branch from 4.7.0 to master June 25, 2020 22:32
@samhouts samhouts added breaking Changes behavior or appearance deprecation Public API has been deprecated t/enhancement ➕ API-change Heads-up to reviewers that this PR may contain an API change a/shell 🐚 labels Jun 25, 2020
@samhouts samhouts removed this from In Review in 4.7.0 Jun 25, 2020
@samhouts samhouts changed the base branch from master to 4.8.0 June 25, 2020 23:58
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

Build failed!

@jsuarezruiz
Copy link
Contributor Author

Fixed the build.

@samhouts samhouts added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Jul 16, 2020
@samhouts samhouts added this to Ready for Review (PRs) in Sprint 173 Jul 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/layout a/shapes a/shell 🐚 API-change Heads-up to reviewers that this PR may contain an API change blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. breaking Changes behavior or appearance ControlGallery Core deprecation Public API has been deprecated p/Android p/iOS 🍎 p/UWP p/WPF t/enhancement ➕
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants