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

Fixes #10402 - GetCropUrl Url extension has duplicate signatures and results in error #10524

Closed
wants to merge 1 commit into from

Conversation

nul800sebastiaan
Copy link
Member

This is a breaking change for people using GetCropUrl to get local crops from the media picker v3. In order to disambiguate the extension method an make it a bit clearer what it does, I propose the method name change, after which there is no need to have the first argument being the local crops, but it can be the whole MediaWithCrops object, since the method name already makes clear you're not trying to get the global crops.

Fixes #10402

…results in error

This is a breaking change for people using `GetCropUrl` to get local crops from the media picker v3. In order to disambiguate the extension method an make  it a bit clearer what it does, I propose the method name change, after which there is no need to have the first argument being the local crops, but it can be the whole MediaWithCrops object, since the method name already makes clear you're not trying to get the global crops.
@ronaldbarendse
Copy link
Contributor

@nul800sebastiaan Changing the method parameter types would indeed be a breaking change, but this already uses a non-standard default value for bool useCropDimensions = true (all other overloads with this parameter have it set to false).

I've created PR #10527 to align the overloads on UrlHelper with those on IPublishedContent, which would be a better fix IMHO 😇

@nul800sebastiaan
Copy link
Member Author

Closing in favor of the upcoming changes in 8.15 (#10529).

@nul800sebastiaan nul800sebastiaan deleted the v8/fix/10402 branch June 28, 2021 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GetCropUrl Url extension has duplicate signatures and results in error
2 participants