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.

Sign up for GitHub

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

Jump to bottom

NEW: New extension point for has one and many fields. #17

Merged
merged 1 commit into from
May 20, 2020
Merged

NEW: New extension point for has one and many fields. #17

merged 1 commit into from
May 20, 2020

Conversation

mfendeksilverstripe
Copy link
Contributor

@mfendeksilverstripe mfendeksilverstripe commented May 19, 2020 •
edited
Loading

New extension point for has one and many field

  • this provides better configuration options as existing extension point updateField is not called via handleRequest
  • new extension points are also more specific so it's easier to target customisation

Example:

  • I don't want to have auto complete component for has one field as links can't be shared between objects in my project

Related issues

@mfendeksilverstripe mfendeksilverstripe mentioned this pull request May 19, 2020
Open
elliot-sawyer
elliot-sawyer reviewed May 19, 2020
View reviewed changes
Copy link
Owner

@elliot-sawyer elliot-sawyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is just adding an extension hook before returning, so its addition will not impact on other users of the module. Seems OK to me. @gorriecoe do you have any thoughts?

@gorriecoe
Copy link
Contributor

Looks good to me

@gorriecoe gorriecoe merged commit fdd3723 into elliot-sawyer:master May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elliot-sawyer elliot-sawyer elliot-sawyer left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mfendeksilverstripe @gorriecoe @elliot-sawyer