Feature/1.x/area and ux #24

ekes · 2021-06-16T09:10:29Z

Provides

New geo entity type for 'areas'

Areas have a title and a shape.
The shape can be uploaded using kml, gpx, wkt or geojson

Updates 'address' type

Now includes a title, by default automatically generated by turning the address field into a string of all the sub-fields in it.
Upgrade path for existing address geo entities to populate their title (run db updates).
Title can be made editable, or have a different default with tokens, if desired.
Address geocoding look-up now done from the address field itself by pressing a button¹

Entity Browser

Provides a new entity browser to:

Reuse existing locations. They can be searched by title (so part of address, or the name given to an area).
Create new address or area locations inline.

Installation

With composer, or manually making sure that geocoder ajax prepoulate module is on the issue branch that fixes it for inline entity reference

Test

Use with feature/2.x/localgov-geo-browser branch of LocalGov Directories which has Venue reconfigured to use the entity browser for addresses;
Or manually 'Manage Form Display' of any location field to use Entity browser: Geo Browser (optionally with settings Selection mode: Append to selection, Entity display: Rendered entity, 'View Mode: Default`)

Enable/Disable bundles 'address' and 'area' on the allowed referenced bundles on the field as desired: ie you can have a field that may only target addresses, or areas, or one that can use both.

…button.

Originally the thought had been to have a calculated label, but it makes more sense to have something calculated once, and a field that can be used to filter/search in views and search_api. Default token for address bundle is included, and in the update. An area #1 bundle might want a manually written label?

Split address implementation into separate submodule mainly for better testing.

Originally the thought had been to have a calculated label, but it makes more sense to have something calculated once, and a field that can be used to filter/search in views and search_api. Default token for address bundle is included, and in the update. An area #1 bundle might want a manually written label?

Split address implementation into separate submodule mainly for better testing.

…onfiguration.

…eature/1.x/area-and-ux

Works better ootb certainly with some geocoders. Ideal world would mix auto complete on the address field with selecting the location on the map.

ekes · 2021-06-16T09:12:57Z

¹ This can be even better. Longer term aim to provide button or autocomplete on the address field to show a list of results from geocoder that can then be selected to update the point on the map. But as an incremental change the button is already an improvement in UX.

ekes · 2021-06-18T10:42:47Z

@willguv This is on test.localgovdrupal.org

willguv · 2021-06-18T10:59:17Z

Thanks @ekes. I guess we need some acceptance tests for these, but here's a couple of things I've spotted

When creating a new location:

I did a search for "king" and it didn't seem to narrow down the results
I tried to create a new address, added "1 Grove Road, Eastbourne, BN21 4UG" and clicked "populate from address field. The pin was dropped somewhere in the Atlantic under Africa (prob with Open Street Map?)
do you have any kml files to hand so I can try creating an area? I can't find the one I gave you - sorry

When editing an existing location:

The title bar reads "Edit entity [address]". Can you remove the word "entity"
The "enabled" boolean. Is there an reason why it might not be enabled? Wondering if this is extra info that content designers won't need and so can be hidden for them

ekes · 2021-06-18T11:14:57Z

I did a search for "king" and it didn't seem to narrow down the results

I did that. United Kingdom is returned in all those searches.

I tried to create a new address, added "1 Grove Road, Eastbourne, BN21 4UG" and clicked "populate from address field. The pin was dropped somewhere in the Atlantic under Africa (prob with Open Street Map?)

Yes that's Null Island. (0, 0) No result.

My follow-up #24 (comment) will make that even more explicit (like showing what there is when there is).

One thing to try, just to get a feel for what there is now is try just Eastbourne first; or Grove Road etc.

do you have any kml files to hand so I can try creating an area? I can't find the one I gave you - sorry

The ones from Croydon localgovdrupal/localgov_directories#14 (comment)
Would be good to try some different ones too though.

The title bar reads "Edit entity [address]". Can you remove the word "entity"

Should be configurable somewhere.

The "enabled" boolean. Is there an reason why it might not be enabled? Wondering if this is extra info that content designers won't need and so can be hidden for them

Certainly configurable. We're not really using it either.

ekes · 2021-06-18T19:00:31Z

I tried to create a new address, added "1 Grove Road, Eastbourne, BN21 4UG" and clicked "populate from address field.
The pin was dropped somewhere in the Atlantic under Africa (prob with Open Street Map?)

Yes that's Null Island. (0, 0) No result.
My follow-up #24 (comment) will make that even more explicit (like showing what there is when there is).
One thing to try, just to get a feel for what there is now is try just Eastbourne first; or Grove Road etc.

However it should have found that particular address, even with openstreet map nominatim.

https://nominatim.openstreetmap.org/ui/search.html?q=1+Grove+Road%2C+Eastbourne%2C+BN21+4UG
(or for the gory details of how it works https://nominatim.openstreetmap.org/search.php?q=1+Grove+Road%2C+Eastbourne%2C+BN21+4UG&format=jsonv2&debug=1 )

And it does on my local copy. test.localgov doesn't seem to be responding at the moment. I'll debug why it's not doing it there when I can. Also when it doesn't find a result it displays to the user 'Couldn't find a result for xxxxxxx' (where xxxxxx is the address it searched for).

ekes · 2021-06-22T09:42:32Z

However it should have found that particular address, even with openstreet map nominatim.

https://test.localgovdrupal.org/admin/content/geo/15

This seems to be something about the config that's ended up on test. It does seem to be geocoding the address (see link above), and the Lon Lat is updated, but the visualisation on the map on the form isn't.

stephen-cox · 2021-06-30T13:36:58Z

@ekes @willguv I have redeployed the https://test.localgovdrupal.org/ site so we can test the workflow stuff. I have redeployed this work as well, but any manual changes will have been lost.

finnlewis · 2021-07-07T11:32:47Z

@finnlewis take a look!

willguv · 2021-07-23T12:57:08Z

Hi @ekes

I'm checking this today and will add a few thoughts as I go - mostly niggly things now

On selecting a location, the checkboxes should align with each address rather than sit on top

When zooming in and out of the map, is it possible to configure the centre of the zoom on the place where the pin is dropped? Neither entry I've added has populated from the address field, so having to zoom out, drop the pin, and zoom back in, continually moving the screen back to the pin is very time consuming!
I'd like to add an area but that option isn't available on test? I'm getting this error when trying on demo

The website encountered an unexpected error. Please try again later.

Symfony\Component\Routing\Exception\RouteNotFoundException: Route "view.localgov_approvals_dashboard.approvals_dashboard" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 206 of core/lib/Drupal/Core/Routing/RouteProvider.php).
Drupal\Core\Menu\LocalTaskDefault->getRouteParameters(Object) (Line: 310)
Drupal\Core\Menu\LocalTaskManager->getTasksBuild('system.admin_content', Object) (Line: 358)
Drupal\Core\Menu\LocalTaskManager->getLocalTasks('system.admin_content', 0) (Line: 95)
Drupal\Core\Menu\Plugin\Block\LocalTasksBlock->build() (Line: 171)
Drupal\block\BlockViewBuilder::preRender(Array)
call_user_func_array(Array, Array) (Line: 101)
Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 786)
Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array) (Line: 377)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 449)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 201)
Drupal\Core\Render\Renderer->render(Array) (Line: 450)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 47)
__TwigTemplate_ae6a3abc1013b119c6ac8327b299a71950d89b3c76c1bac392ecd705c6b2021a->doDisplay(Array, Array) (Line: 405)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 378)
Twig\Template->display(Array) (Line: 390)
Twig\Template->render(Array) (Line: 65)
twig_render_template('core/themes/claro/templates/page.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('page', Array) (Line: 436)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 201)
Drupal\Core\Render\Renderer->render(Array) (Line: 450)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 86)
__TwigTemplate_5d0b1ec6d5027e060b923f82053946012d4b50de2df7025c01446d6ec1c5d269->doDisplay(Array, Array) (Line: 405)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 378)
Twig\Template->display(Array) (Line: 390)
Twig\Template->render(Array) (Line: 65)
twig_render_template('core/themes/claro/templates/classy/layout/html.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('html', Array) (Line: 436)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 201)
Drupal\Core\Render\Renderer->render(Array) (Line: 162)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent{closure}() (Line: 578)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 163)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
call_user_func(Array, Object, 'kernel.view', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.view') (Line: 163)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 80)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 716)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Adnan-cds · 2021-07-30T10:31:13Z

I am reviewing the code. Will is facing PHP exceptions above. Things must be missing. I am checking the functioning part :)

Adnan-cds

As Steve has already pointed out, several config files have a UUID value in them :(

Adnan-cds · 2021-07-30T16:18:31Z

Impressive and fun to use. I didn't get any Twig error either :)

Few small issues:

While adding a new Address Geo entity, the coordinates grabbed by "Populate from the Address field" button from the geocoder_ajax_prepopulate module is off by a few miles! For example, I tried "CR0 1EA" as the postcode with the postal town set to "Croydon". The resulting coordinate is different from what I get at a different service. This button also doesn't immediately update the map leaving the marker in the middle of the ocean. Will may have reported this already. Once I save the form, the map show fine albeit a few miles off :(
/admin/content/geo doesn't include the bundle of the Geo entity. Now that we have at least two bundles, knowing the bundle should help UX. It's a two liner, so I thought why not? Patch follows:

diff --git a/src/LocalgovGeoListBuilder.php b/src/LocalgovGeoListBuilder.php
index 8069944..bf47cc5 100644
--- a/src/LocalgovGeoListBuilder.php
+++ b/src/LocalgovGeoListBuilder.php
@@ -79,6 +79,7 @@ class LocalgovGeoListBuilder extends EntityListBuilder {
    */
   public function buildHeader() {
     $header['id'] = $this->t('ID');
+    $header['bundle'] = $this->t('Type');
     $header['status'] = $this->t('Status');
     $header['uid'] = $this->t('Author');
     $header['created'] = $this->t('Created');
@@ -91,6 +92,7 @@ class LocalgovGeoListBuilder extends EntityListBuilder {
    */
   public function buildRow(EntityInterface $entity) {
     $row['id'] = $entity->toLink();
+    $row['bundle'] = $entity->get('bundle')->entity->label();
     $row['status'] = $entity->isEnabled() ? $this->t('Enabled') : $this->t('Disabled');
     $row['uid']['data'] = [
       '#theme' => 'username',

Lastly

you can have a field that may only target addresses, or areas, or one that can use both.

By default, Directory venue has only the Address bundle of the localgov_geo entity enabled. But the entity browser lists both Area and Address. If I select an "Area" and then try to save the Directory venue, it comes up with a form error. Thankfully the fix was easy. Here's a patch:

diff --git a/config/install/views.view.localgov_geo_library.yml b/config/install/views.view.localgov_geo_library.yml
index 284f7b5..1675dc1 100644
--- a/config/install/views.view.localgov_geo_library.yml
+++ b/config/install/views.view.localgov_geo_library.yml
@@ -1,4 +1,3 @@
-uuid: 8a21de69-d08b-4663-9bf8-012f06ab31d3
 langcode: en
 status: true
 dependencies:
@@ -241,7 +240,50 @@ display:
       footer: {  }
       empty: {  }
       relationships: {  }
-      arguments: {  }
+      arguments:
+        bundle:
+          id: bundle
+          table: localgov_geo_field_data
+          field: bundle
+          relationship: none
+          group_type: group
+          admin_label: ''
+          default_action: default
+          exception:
+            value: all
+            title_enable: false
+            title: All
+          title_enable: false
+          title: ''
+          default_argument_type: entity_browser_widget_context
+          default_argument_options:
+            context_key: target_bundles
+            fallback: all
+            multiple: or
+          default_argument_skip_url: false
+          summary_options:
+            base_path: ''
+            count: true
+            items_per_page: 25
+            override: false
+          summary:
+            sort_order: asc
+            number_of_records: 0
+            format: default_summary
+          specify_validation: false
+          validate:
+            type: none
+            fail: 'not found'
+          validate_options: {  }
+          glossary: false
+          limit: 0
+          case: none
+          path_case: none
+          transform_dash: false
+          break_phrase: true
+          entity_type: localgov_geo
+          entity_field: bundle
+          plugin_id: string
       display_extenders: {  }
     cache_metadata:
       max-age: -1

Please note that I have tried this on a fresh localgov install. I haven't tried the update hooks on an existing site.

That's all.

Adnan-cds · 2021-08-04T13:28:12Z

One last point: The repository config added to composer.json only takes effect in root level composer.json files AFAIK. So either this needs to be added to the root level composer.json or localgov_geo/composer.json should refer to a patch.

ekes · 2021-08-09T17:08:31Z

While adding a new Address Geo entity, the coordinates grabbed by "Populate from the Address field" button from the geocoder_ajax_prepopulate module is off by a few miles! For example, I tried "CR0 1EA" as the postcode with the postal town set to "Croydon". The resulting coordinate is different from what I get at a different service.

I'm wondering if we should package with a default geocoder at all. This seems to be the source of the greatest number of bug reports and the intention is not that you actually use the default Nominatim geocoder. We could maybe set up one for the demo site with API keys etc. but it would mean that the demo install wouldn't work out of the box.

Although it might be worth looking a what gets sent to Nominatim, because https://nominatim.openstreetmap.org/ui/search.html?q=CR0+1EA+UK https://nominatim.openstreetmap.org/ui/search.html?q=CR0+1EA More about that in the next comment.

For reference the geocoders supported are https://packagist.org/providers/geocoder-php/provider-implementation if there's another common one used by Local Government that's not there we should be able to create one; but I think it covers most bases!

ekes · 2021-08-09T17:17:49Z

One last point: The repository config added to composer.json only takes effect in root level composer.json files AFAIK. So either this needs to be added to the root level composer.json or localgov_geo/composer.json should refer to a patch.

If we get this in before the patch itself https://www.drupal.org/project/geocoder_ajax_prepopulate/issues/3182650#comment-14060179 (which is just waiting on a second review I think) we'll have to include it in the distro composer.json too.

But
(a) we could just get the module patch reviewed
(b) while I want to get this patch in rather than making it even bigger I've been working on an 'autocomplete' address that lists the suggestions as people type. It adds autocompletion to the whole address field, and will fill in the values for the whole address field and populate the position on the map. This is a pain to code generically, but as we have our own entity we know which fields they are. I find this makes it easier for people to know what is possibly being returned, and it ties into completing the adddress field too \o/ But I'll tackle the other small issues on this one first, because I think fixing the UX on geocoding and reusing locations is pretty important - Lambeth have this in prod for ages now.

ekes · 2021-08-09T17:22:50Z

I'd like to add an area but that option isn't available on test? I'm getting this error when trying on demo
The website encountered an unexpected error. Please try again later.
Symfony\Component\Routing\Exception\RouteNotFoundException: Route "view.localgov_approvals_dashboard.approvals_dashboard"

Looks like something to do with workflow isn't working, not related to this patch. Guess that's already going in, and we can deploy again.

ekes · 2021-08-25T16:09:28Z

Going to close this in favour of #29, but if folks don't like the address autocomplete we can always step back to here.

…#24

finnlewis and others added 14 commits November 20, 2020 17:29

Change UX for geocoding an address, defaulting to use Open Street Map.

f22738b

Rmove default_config_hash keys from config install.

176980e

Resolve merge conflict and coding standards issues.

5db0843

Adding drupal/geocoder_ajax_prepopulate for the geocode from address …

f3ae24a

…button.

Repository for f3ae24a.

a06e4f4

Add basic tests for type and address entity.

67709ba

Split address implementation into separate submodule mainly for better testing.

Add basic tests for type and address entity.

459ec1d

Split address implementation into separate submodule mainly for better testing.

WIP dependencies and tests.

3a1b75b

Work-around to display correct entity creation forms based on field c…

ed745a1

…onfiguration.

Merge remote-tracking branch 'origin/feature/1.0-improvements' into f…

21bbeb0

…eature/1.x/area-and-ux

Merge branch 'feature/1.x/1-area' into feature/1.x/area-and-ux

1029bdf

Use prepopulate from address field.

5404264

Works better ootb certainly with some geocoders. Ideal world would mix auto complete on the address field with selecting the location on the map.

finnlewis self-requested a review July 7, 2021 11:32

finnlewis self-assigned this Jul 16, 2021

finnlewis requested review from willguv, andybroomfield, Adnan-cds and stephen-cox and removed request for finnlewis July 20, 2021 10:37

finnlewis added this to In progress in Beta phase via automation Jul 20, 2021

finnlewis moved this from In progress to Review in progress in Beta phase Jul 20, 2021

Adnan-cds requested changes Jul 30, 2021

View reviewed changes

finnlewis mentioned this pull request Aug 10, 2021

Demo site has a problem localgovdrupal/localgov#306

Closed

This was referenced Aug 18, 2021

Addressfield Geocoder UX #27

Closed

Geo Areas PR #28

Closed

Area geo bundle #1

Closed

ekes mentioned this pull request Aug 25, 2021

Feature/1.x/area and ux autocomplete #29

Merged

ekes closed this Aug 25, 2021

Beta phase automation moved this from Review in progress to Done - Sprint 6 Aug 25, 2021

stephen-cox added a commit that referenced this pull request May 31, 2022

Unpin Leaflet to install latest version #24

51d2509

stephen-cox added a commit that referenced this pull request May 31, 2022

Re-exported localgov_geo_address display config for Leaflet 2.2 update …

df48d8c

…#24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/1.x/area and ux #24

Feature/1.x/area and ux #24

ekes commented Jun 16, 2021 •

edited

Loading

ekes commented Jun 16, 2021

ekes commented Jun 18, 2021

willguv commented Jun 18, 2021 •

edited

Loading

ekes commented Jun 18, 2021 •

edited

Loading

ekes commented Jun 18, 2021 •

edited

Loading

ekes commented Jun 22, 2021

stephen-cox commented Jun 30, 2021

finnlewis commented Jul 7, 2021

willguv commented Jul 23, 2021

Adnan-cds commented Jul 30, 2021

Adnan-cds left a comment

Adnan-cds commented Jul 30, 2021

Adnan-cds commented Aug 4, 2021

ekes commented Aug 9, 2021 •

edited

Loading

ekes commented Aug 9, 2021

ekes commented Aug 9, 2021

ekes commented Aug 25, 2021

Feature/1.x/area and ux #24

Feature/1.x/area and ux #24

Conversation

ekes commented Jun 16, 2021 • edited Loading

Provides

New geo entity type for 'areas'

Updates 'address' type

Entity Browser

Installation

Test

ekes commented Jun 16, 2021

ekes commented Jun 18, 2021

willguv commented Jun 18, 2021 • edited Loading

ekes commented Jun 18, 2021 • edited Loading

ekes commented Jun 18, 2021 • edited Loading

ekes commented Jun 22, 2021

stephen-cox commented Jun 30, 2021

finnlewis commented Jul 7, 2021

willguv commented Jul 23, 2021

Adnan-cds commented Jul 30, 2021

Adnan-cds left a comment

Choose a reason for hiding this comment

Adnan-cds commented Jul 30, 2021

Adnan-cds commented Aug 4, 2021

ekes commented Aug 9, 2021 • edited Loading

ekes commented Aug 9, 2021

ekes commented Aug 9, 2021

ekes commented Aug 25, 2021

ekes commented Jun 16, 2021 •

edited

Loading

willguv commented Jun 18, 2021 •

edited

Loading

ekes commented Jun 18, 2021 •

edited

Loading

ekes commented Jun 18, 2021 •

edited

Loading

ekes commented Aug 9, 2021 •

edited

Loading