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

Clean add this element add this wrapper #40

Merged

Conversation

ChuChuNaKu
Copy link

No description provided.

@nerdstein
Copy link
Contributor

@ChuChuNaKu - which issue are you referencing? please provide with all PRs

),
),
);
//// $element['addthis_basic_toolbox'] = array(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why this is commented out? Plan to use later? If not, remove

@nerdstein
Copy link
Contributor

Reviewed and really impressed with how much cleanup occured in this PR, great job. I'll review with more context when I get the issue

@ChuChuNaKu
Copy link
Author

@nerdstein sorry about that. This is for issue number #16. I had LOTS of help!


return array(
'#markup' => $markup
'#type' => 'addthis_basic_toolbox',
'#size' => $config['basic_toolbox']['buttons_size'],
Copy link
Contributor

Choose a reason for hiding this comment

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

$config does not exist in the context of this function. Blocks use $this->configuration while Fields use $this->getSettings(). You will need to adjust these for the fields.

@ChuChuNaKu
Copy link
Author

ok, @nerdstein I got rid of the commented code. @doylejd I reworked the fields.

@doylejd
Copy link
Contributor

doylejd commented Aug 21, 2015

@ChuChuNaKu Going to merge this so Jason can get his stuff working, but I am pretty sure the config arrays are wrong in the field formatters. Lets get a new PR open for that.

doylejd pushed a commit that referenced this pull request Aug 21, 2015
@doylejd doylejd merged commit 526737c into d8-contrib-modules:master Aug 21, 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