Upgrade Atk UI develop, future version 4.0 #13
Conversation
|
@abbadon1334 - Before recent Js changes, registering a plugin automatically append the 'atk' prefix to the plugin name. I think this is not the case anymore therefore your changes should work correctly. |
src/Utils/JsLoader.php
Outdated
| @@ -17,7 +17,7 @@ class JsLoader | |||
| public static $cdn = 'https://cdn.jsdelivr.net/gh/atk4/google-address'; | |||
|
|
|||
| /** @var string Javascript file version. */ | |||
| public static $version = '2.2.2'; | |||
| public static $version = '4.0.0'; | |||
There was a problem hiding this comment.
remove it, no CDN, JS files are versioned together with atk4/ui files
There was a problem hiding this comment.
remove CDN and set the actual as default fallback: /public/atk-google-maps.min.js but that path is unusable in a real env, so I change it /assets/atk-google-maps.min.js
| @@ -16,19 +16,19 @@ | |||
| class AddressLookup extends Line | |||
| { | |||
| /** @var array An array of Google address_components to fill specific controls with. */ | |||
| public $controlMap = []; | |||
| public array $controlMap = []; | |||
There was a problem hiding this comment.
either remove @var array phpdoc or revert
There was a problem hiding this comment.
Add typed array in PHPDoc, I think is better
| @@ -106,7 +105,7 @@ public function renderView(): void | |||
| { | |||
| JsLoader::load($this->getApp()); | |||
|
|
|||
| $this->js(true)->atkAddressLookup($this->getLookupSettings()); | |||
| $this->js(true)->AddressLookup($this->getLookupSettings()); | |||
There was a problem hiding this comment.
where does the AddressLookup JS comes from? In atk4/ui AFAK there is no such plugin
There was a problem hiding this comment.
here in the repo, there is the js source
| @@ -10,14 +10,16 @@ | |||
| * address lookup plugin concatenating each property using the specified | |||
| * glue in order to set input value. | |||
| */ | |||
| class Build | |||
| final class Build | |||
There was a problem hiding this comment.
the internal call to static gives a stan error, and this class is not intended to be reused so i think is the easy way to avoid it
There was a problem hiding this comment.
even this has the same stan error and the same future use as Build class : https://github.com/abbadon1334/atk4-google-address/blob/cb472c3c3197e26ce0a724771ed51bcb25183c06/src/Utils/Value.php#L7
final is the best option, those classes are intended for internal usage strictly related to GoogleAPI
There was a problem hiding this comment.
I prefer non final, final is sometimes very hard to overcome to workaround some problems on user side.
| if: matrix.type == 'StaticAnalysis' | ||
| run: | | ||
| echo "memory_limit = 2G" > /usr/local/etc/php/conf.d/custom-memory-limit.ini | ||
| vendor/bin/phpstan analyse |
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "atk-google-address", | |||
| "version": "2.2.0", | |||
| "version": "4.0.0", | |||
There was a problem hiding this comment.
should be 1.0.0 as in https://github.com/atk4/ui/blob/fadc7680ed16ce15267968524bbc3217c8ea323b/js/package.json#L3
JS is versioned using composer (and git)
| @@ -15,20 +15,20 @@ | |||
| */ | |||
| class AddressLookup extends Line | |||
| { | |||
| /** @var array An array of Google address_components to fill specific controls with. */ | |||
| public $controlMap = []; | |||
| /** @var list<array{name:string,value:Build}> An array of Google address_components to fill specific controls with. */ | |||
There was a problem hiding this comment.
| /** @var list<array{name:string,value:Build}> An array of Google address_components to fill specific controls with. */ | |
| /** @var list<array{name:string, value:Build}> An array of Google address_components to fill specific controls with. */ |
|
|
||
| if (!self::$apiKey) { | ||
| if (self::$apiKey === '') { |
There was a problem hiding this comment.
if we define prop like protected static string $apiKey;, ex. is not needed with typed property :)
| /** @var bool */ | ||
| private static $isLoaded = false; | ||
| /** Default location of js file, use this if $locationUrl is not specified in JsLoader::load($app, '/new/path/to/js-file.min.js'); */ | ||
| protected static string $defaultLocationUrl = '/assets/atk-google-maps.min.js'; |
There was a problem hiding this comment.
| /** @var string Google maps version. */ | ||
| protected static $apiVerstion = 'quarterly'; | ||
| /** Google maps version. */ | ||
| protected static string $apiVerstion = 'quarterly'; |
|
|
||
| public static function setGoogleApiKey(string $key): void | ||
| { | ||
| self::$apiKey = $key; | ||
| } | ||
|
|
||
| /** | ||
| * @param array<string, int|string|string[]|mixed> $options |
There was a problem hiding this comment.
| * @param array<string, int|string|string[]|mixed> $options | |
| * @param array<string, mixed> $options |
mixed is always what phpstan reads in both cases :)
|
|
||
| /** @var string Google Map property to use. (long_name or short_name) */ | ||
| private $property; | ||
| /** Google Map property to use. (long_name or short_name) */ |
There was a problem hiding this comment.
shortName or is it some G API name?
@mvorisek I think the best option is to release the actual dev as a 2.3 version and use this as the new develop
@ibelar
I need to change this line :
https://github.com/abbadon1334/atk4-google-address/blob/de791a14313a72618eb29080159f640e472cf780/src/Form/Control/AddressLookup.php#L108
I get a javascript error
atkAddressLookup is not defined functionchanging the init in that file toAddressLookupit works as excepted. Am i missing something or is the right change to do?i changed even the version to be recovered from cdn.jsdelivr.net to 4.0.0, the file needs to be uploaded?