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

add google places helper #36

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mh-SimTEC
Copy link

Add a helper for google places and add libraries options to google Map helper

I don't know if i have fully understand how you include the google api.

Add a helper for google places and add libraries options to google Map helper
*
* @return string divContainer
*/
public function input( $fieldName, array $fieldOptions = [], $googleOptions = [] ) {
Copy link
Owner

Choose a reason for hiding this comment

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

Is this for BC?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I would like to use the same methods as FormHelper
I can remove this or add @deprecated

Copy link
Owner

Choose a reason for hiding this comment

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

Adding the tag suffices :)

*
* @return string divContainer
*/
public function control( $fieldName, $fieldOptions = [], $googleOptions = [] ) {
Copy link
Owner

Choose a reason for hiding this comment

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

arrays should have typehints
also please check CS issues

* @param $id string the id of the input field
* @param array $options associative array of settings for places.Autocomplete
*/
private function _script( $id, $options = [] ) {
Copy link
Owner

Choose a reason for hiding this comment

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

protected and array typehint

//http://ajax.googleapis.com/ajax/libs/jquery/1/jquery.min.js
}
// still not very common: http://code.google.com/intl/de-DE/apis/maps/documentation/javascript/basics.html
if (false && !empty($this->_runtimeConfig['autoScript']) && !$this->_gearsIncluded) {
Copy link
Owner

Choose a reason for hiding this comment

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

if false => we can remove this part then?

@@ -112,6 +111,8 @@ function initialize() {

$script = 'jQuery(document).ready(function() {' . $js . '});';

$this->Html->scriptBlock($script, ['block' => true]);
$this->Html->scriptBlock( $script, [ 'block' => true ] );
Copy link
Owner

Choose a reason for hiding this comment

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

You should run the sniffer instead as documented, this is all invalid CS :)

@dereuromark
Copy link
Owner

dereuromark commented Dec 20, 2017

Sure you can the composer cs-check and cs-fix commands?

now with the correct standard
@mh-SimTEC
Copy link
Author

i had included codesniffer standard psr2 into phpStorm. the composer commands are new for me. Looks like these are other standards.
Finally done :)

* @return string divContainer
*/
public function control($fieldName, array $fieldOptions = [], array $googleOptions = []) {
$id = isset($fieldOptions['id']) && $fieldOptions['id'] != '' ? $fieldOptions['id'] : $fieldName;
Copy link
Owner

Choose a reason for hiding this comment

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

!empty(fieldOptions['id']) is shorter than both checks. 0 is not a valid id anyway right?


$html = $this->Form->control($fieldName, $fieldOptions);
$html .= $this->Form->hidden("{$fieldName}_lat", ['id' => "{$id}_lat"]);
$html .= $this->Form->hidden("{$fieldName}_lon", ['id' => "{$id}_lon"]);
Copy link
Owner

Choose a reason for hiding this comment

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

lng is usually used for longitude shortform in this plugin.

@dereuromark
Copy link
Owner

Can we have a small test for the new functionality and helper?
Other than that this looks good to go - thx for the new feature.

@mh-SimTEC
Copy link
Author

I will have a look at your cakephp-sandbox and build some example later at home.

@dereuromark
Copy link
Owner

Any update here?

@dereuromark
Copy link
Owner

ping @mh-SimTEC

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.

None yet

2 participants