-
Notifications
You must be signed in to change notification settings - Fork 475
[Enhancement] AutoFitLabel #1164
[Enhancement] AutoFitLabel #1164
Conversation
if (Element is null) | ||
return; | ||
|
||
#pragma warning disable 618 // We will need to update this when .Font goes away |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls add // TODO:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And do we actually need to use obsolete API? Can we use new props?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say that we don't want to use Obsolete APIs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@felipebaltazar thanks for this PR. I know this is still a draft PR, so I just did comments regarding the current code, to win time.
static BuildVersionCodes? sdkInt; | ||
static bool? isLollipopOrNewer; | ||
static bool? isNougatOrNewer; | ||
|
||
static BuildVersionCodes SdkInt => | ||
sdkInt ??= Build.VERSION.SdkInt; | ||
|
||
static bool IsNougatOrNewer => | ||
isNougatOrNewer ??= SdkInt >= BuildVersionCodes.N; | ||
|
||
static bool IsLollipopOrNewer => | ||
isLollipopOrNewer ??= SdkInt >= BuildVersionCodes.Lollipop; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's move those bits into another file
|
||
bool hasLayoutOccurred; | ||
bool wasFormatted; | ||
bool disposed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool disposed; | |
bool isDisposed; |
var oldElement = element; | ||
element = value; | ||
|
||
if (oldElement != null && element != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we create the control for the first time, the oldElement
will be null
and the OnElementChanged
will not be triggered, and it should be triggered because the element
changed.
|
||
result.Minimum = new Size(Math.Min(Context.ToPixels(10), result.Request.Width), result.Request.Height); | ||
|
||
var measureIsChanged = !lastSizeRequest.HasValue || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's store the lastSizeRequest.HasValue
and lastSizeRequest.Value
into a local variable and reuse it.
void IVisualElementRenderer.SetElement(VisualElement element) | ||
{ | ||
if (element is not AutoFitLabel label) | ||
throw new ArgumentException("Element must be of type Label"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw new ArgumentException("Element must be of type Label"); | |
throw new ArgumentException($"{nameof(element)} must be of type Label"); |
} | ||
} | ||
|
||
void SetLineBreakMode(Label label) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the Forms methods? For this and other methods below
Please create an extension file with those methods, just like Forms did
https://github.com/xamarin/Xamarin.Forms/blob/5.0.0/Xamarin.Forms.Platform.Android/Extensions/TextViewExtensions.cs
} | ||
} | ||
|
||
static class BackgroundManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this class to another file?
UpdateBackgroundColor(control, element); | ||
} | ||
|
||
Performance.Stop(reference); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you remove this and any other call to Performance
member?
{ | ||
} | ||
|
||
[Obsolete] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to implement these Obsolete members?
void UpdateAutoFitMode() | ||
{ | ||
#if __MOBILE__ | ||
if (Element is AutoFitLabel autoFitLabel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you invert this if
?
@felipebaltazar are you able to move this PR foward? |
Oh, yeah! Sorry for the delayed response... I'm a little busy this month... |
Thanks! However, we are no longer adding new features to Xamarin Community Toolkit, focusing on the .NET MAUI Community Toolkit. Please open a New Feature Discussion to implement this feature in the .NET MAUI Community Toolkit. I've posted more information about the Future Of Xamarin Community Toolkit here: https://devblogs.microsoft.com/xamarin/the-future-of-xamarin-community-toolkit/?WT.mc_id=mobile-0000-bramin |
Description of Change
Bugs Fixed
API Changes
Added:
Behavioral Changes
Ive implemented a label that adjusts font size according of the label`s size
(I will include some images about this feature)
PR Checklist
approved