-
Notifications
You must be signed in to change notification settings - Fork 824
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
NEW Provide a standardised CMSEditLink method #11338
NEW Provide a standardised CMSEditLink method #11338
Conversation
@@ -4143,6 +4153,7 @@ public static function enable_subclass_access() | |||
*/ | |||
private static $casting = [ | |||
"Title" => 'Text', | |||
'CMSEditLink' => 'Text', |
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.
Moved from silverstripe/silverstripe-cms#2987
c7173df
to
147fa90
Compare
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.
Could we change it to getEditLink(), rename the extension hook updateEditLink
and then retain CMSEditLink() as a method that simply calls getEditLink(), deprecate but don't remove it until CMS 7?
Discussed in person - decided not to rename it given we have other CMS related methods in |
147fa90
to
710f1a8
Compare
710f1a8
to
9d2050a
Compare
Provides a standard implementation and method signature for the
CMSEditLink()
method which is used in various places. See related PRs.This change allows us to convert the public
CMSEditLink()
method onCMSEditLinkExtension
to instead be an appropriate protectedupdateCMSEditLink()
extension hook. See silverstripe/silverstripe-admin#1809If we were doing this from scratch I'd call it
getEditLink()
- if you want that change, since this is a major release I can do that, but bear in mind that change introduces upgrade pain with probably 0 value added, so don't ask for it unless you can indicate some value it adds to offset the upgrade pain it'd introduce.Issue