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

Abstracted out BasicButton and BasicToolbox Forms/displays. #22

Merged
merged 5 commits into from
Aug 13, 2015

Conversation

doylejd
Copy link
Contributor

@doylejd doylejd commented Aug 13, 2015

  • Abstracted out the BasicButton and BasicToolbox forms to allow for reuse in field/block
  • Abstracted out displays for BasicButton and BasicToolbox to allow for reuse in field/block
  • Updated Block settings form to allow for Type selection and conditional display of subfields
  • Updated Block/Fields Form and Display functions to call shared functions
  • Hooked up Services validation function for the BasicToolbox form

@doylejd
Copy link
Contributor Author

doylejd commented Aug 13, 2015

Resolves issue #20

* @param array $element
* @param FormStateInterface $form_state
*/
function addThisDisplayElementServicesValidate(array $element, FormStateInterface $form_state)
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to go into a controller class

@nerdstein
Copy link
Contributor

@doylejd - big things from my review:

  1. module files should only contain hooks, all custom callbacks should be in controller classes
  2. The 'AddThis' class itself looks like a potential controller.

Controllers should go into src/Controller

@doylejd
Copy link
Contributor Author

doylejd commented Aug 13, 2015

@nerdstein Added comments and refactored code based on standards.

Investigating Controller classes and the ability to move the validate function. Additional refactoring may be needed. If so, I will open new issues for it.

jenter pushed a commit that referenced this pull request Aug 13, 2015
Abstracted out BasicButton and BasicToolbox Forms/displays.
@jenter jenter merged commit 6acdc07 into d8-contrib-modules:master Aug 13, 2015
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.

3 participants