Fix up relevant CA1416 warnings - Set 3 #26751
Conversation
|
Hey there @Tamilarasan-Paranthaman! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
vishnumenon2684
added
the
partner/syncfusion
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
|
|
||
| // ctx will be null if the width/height of the view is zero | ||
| if (ctx is not null && !TryRender(window, out _)) | ||
| if (!OperatingSystem.IsIOSVersionAtLeast(17)) |
There was a problem hiding this comment.
nit: can we reverse the polarity here :)
It makes it a bit easier to read if the first is the latest version and then the else gets older. If 17 else 16 else 15.
There was a problem hiding this comment.
Also, why do we need this block for ios 17+? It seems to have been around since iOS 10? Can we just use that code? The UIGraphics style is obsolete in iOS 18, but we don't need to use that as the main flag here.
There was a problem hiding this comment.
@mattleibow, it seems that BeginImageContextWithOptions is not supported on iOS 17.0 and later.
As @mattleibow suggested here, should we adopt the new approach for older versions as well, @PureWeen?
public Task<IScreenshotResult> CaptureAsync(UIWindow window)
{
_ = window ?? throw new ArgumentNullException(nameof(window));
using var renderer = new UIGraphicsImageRenderer(window.Bounds.Size);
var resultImage = renderer.CreateImage((UIGraphicsImageRendererContext imageContext) =>
{
if (!TryRender(window, out _))
{
// TODO: test/handle this case
}
});
var screenshotResult = new ScreenshotResult(resultImage);
return Task.FromResult<IScreenshotResult>(screenshotResult);
}
var screenshotResult = new ScreenshotResult(resultImage);
return Task.FromResult<IScreenshotResult?>(screenshotResult);Could you please share your thoughts on this, @PureWeen ?
| UIGraphics.EndImageContext(); | ||
|
|
||
| return image.ImageWithRenderingMode(UIImageRenderingMode.Automatic); | ||
| var renderer = new UIGraphicsImageRenderer(imagesize); |
There was a problem hiding this comment.
For example, here you switched directly to the new way without keeping the old way. I think this is better? Thoughts?
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Description of Change
The CA1416 warnings were already resolved in PR #26593
In this PR, we have divided the file changes from the previous PR into smaller, more manageable groups and added test cases for the affected areas to ensure proper functionality.