Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GetCropUrl() extension methods on UrlHelper #10527

Closed
wants to merge 3 commits into from

Conversation

ronaldbarendse
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #10402.

Description

With the introduction of the new Media picker (v3 - with local crops), a new GetCropUrl() extension method on UrlHelper was added that causes a The call is ambiguous between the following methods or properties compiler error when used with named parameters.

This PR aligns the extension methods/parameters on UrlHelper with those on IPublishedContent. Some code might still need updating, but the impact would be a lot lower (as the new extension method has less parameters that can cause ambiguity) and the change will be a lot easier:

Url.GetCropUrl(Model.Image.LocalCrops, cropAlias: "Test") // Still ambiguous, but that's currently also the case for the IPublishedContent extension method
Url.GetCropUrl(Model.Image.LocalCrops, "Test") // Uses the crop alias and its dimensions, easy fix!
Url.GetCropUrl(Model.Image.LocalCrops, cropAlias: "Test", useCropDimensions: true) // Not ambiguous anymore, but the same as the previous call, a bit more explicit, but still an easy fix!

@ronaldbarendse
Copy link
Contributor Author

The changes in this PR show up a bit weird, but if you look at the separate commits, you'll clearly see I've removed the offending GetCropUrl() overload that caused the ambiguous method compiler error (introduced in #9461) and added a new one to allow passing in local crops.

These commits are also part of PR #10529, but that also contains new features.

@ronaldbarendse
Copy link
Contributor Author

I'll close this PR as #10529 that includes these changes is now merged!

@ronaldbarendse ronaldbarendse deleted the v8/bugfix/urlhelper-getcropurl branch June 29, 2021 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GetCropUrl Url extension has duplicate signatures and results in error
1 participant