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

Switch from geolocation to geo entity. #188

Draft
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

ekes
Copy link
Member

@ekes ekes commented Aug 20, 2024

  • Adds a view mode for contact specifically, chances are it wants to de displayed differently from other inline geo entities.
  • Still uses the library, rather than simple inline form, if only because it's the easiest way of keeping it non-compulsory.

What does this change?

For new installs it swaps out seperate geolocation address lookup with lat lon and address field, for a geo entity reference based on localgov geo as used elsewhere in localgov. Issue: #10

How to test

Install localgov. Make a service page with contact paragraph.

Have we considered potential risks?

This doesn't update existing sites. I'd suggest and (optional) upgrade script/module if that is really needed. It does mean there will be a difference between installed fields before after without enforcing changing.

 * Adds a view mode for contact specifically, chances are it wants to de
   displayed differently from other inline geo entities.
 * Still uses the library, rather than simple inline form, if only
   because it's the easiest way of keeping it non-compulsory.
@ekes
Copy link
Member Author

ekes commented Aug 20, 2024

Ah! But ... the change of composer.json and .info.yml dependencies. Hmm....

@andybroomfield
Copy link
Contributor

I would suggest offering this as a new paragraph type may be a simpler approach. We could then deprecate the old one. Note: there may be legitamate reasons why a non geo location paragraph may be in use.

@ekes
Copy link
Member Author

ekes commented Aug 21, 2024

Then we'd have the worst of both worlds, we'd still be installing both paragraphs. At least removing it we only install one for new sites. Plus we'd still have the dependency on both geolocation and geofield.

I just can't think of a simple way of maintaining the geolocation dependency for existing sites, while removing it for new ones.

@ekes
Copy link
Member Author

ekes commented Aug 22, 2024

While I'm editing this contact paragraph. Who is using it. I'm a bit confused by the fields @Adnan-cds I guess Croydon was using it?

Like it has a 'description' section but that just has a heading and subheading, not text area field. But why would you want a heading and subheading on a contact?

@Adnan-cds
Copy link
Contributor

But why would you want a heading and subheading on a contact?

The heading field serves as the heading. The subheading field isn't used often. But I have seen two instances where the subheading has been used like a tagline. You will find an example towards the bottom of this page.

@ekes
Copy link
Member Author

ekes commented Aug 22, 2024

The heading field serves as the heading.

And the label then?

The subheading field isn't used often. But I have seen two instances where the subheading has been used like a tagline.

Think this is where description (as in textarea, rather than single line) was requested at Greenwich.

I guess even though these are components you're not reusing them? Or is the label made to have something that's not the heading, that's also unique to find?

@ekes
Copy link
Member Author

ekes commented Aug 22, 2024

Ohh the tel: on that example page.

@Adnan-cds
Copy link
Contributor

I guess even though these are components you're not reusing them?

We are reusing some. Although most don't appear to be reused. Contact was the primary motivation for activating the paragraphs_library submodule IIRC.

Or is the label made to have something that's not the heading, that's also unique to find?

The label is mostly useful in the component list at /admin/content/paragraphs.

@ekes
Copy link
Member Author

ekes commented Aug 22, 2024

I just can't think of a simple way of maintaining the geolocation dependency for existing sites, while removing it for new ones.

I can't think of a way of doing this without maintaining at least the composer.json download of the code for one (minor?) version while no longer installing the gelocation version of the paragraph. We just don't include it as forced enabled in the (main) .info.yml file. I suppose wizzing these things of into submodules might help?

And then if I'm also adding a description field, and maybe adding more help to the 'label' field, and seeing if we can constrain a field to only allow this paragraph type... perhaps it does became a new contact paragraph type: localgov_contact_new or something?

@Adnan-cds
Copy link
Contributor

I can't think of a way of doing this without maintaining at least the composer.json download

Maybe an update hook with a batch job to copy to the new field type will be simpler.

seeing if we can constrain a field to only allow this paragraph type

Is this a custom field?

@ekes
Copy link
Member Author

ekes commented Aug 23, 2024

seeing if we can constrain a field to only allow this paragraph type

Is this a custom field?

For Greenwich for sure I'll make a contact single value reference field. For LDG generally it might make sense to make one available in place of a all the things component field. Although I understand some councils might want that (in addition) for the service pages. Well it exists already.

@Adnan-cds
Copy link
Contributor

generally it might make sense to make one available in place of a all the things component field.

I am assuming what you are saying is, we will create an entity reference field that references contact paragraphs (or a new alternative) only. This field won't be immediately attached to any content type. If someone wants to use this field in any content type later, they can. Is this assumption correct?

@ekes
Copy link
Member Author

ekes commented Aug 23, 2024

Pretty much where I was headed yep. Greenwich want it on Service pages, Guide pages, and Step-by-Step if I recall correctly; but it would make sense for LGD generally just to make it available I think.

@Adnan-cds
Copy link
Contributor

it would make sense for LGD generally just to make it available I think.

I am okay with this field being available for use across content types.

@ekes
Copy link
Member Author

ekes commented Aug 23, 2024

At which point I ask myself if this should be a paragraph, or not a custom entity type 🤷
I was trying to work with what's there, which is my reluctance, certainly if paragraphs can be reused, and still limited to single types.
I'll keep prodding.

@Adnan-cds
Copy link
Contributor

Interesting. When I woke up this morning, my first thought was, Ekes should just use Geo entities rather than Contact paragraphs :D However, I later realized that Contact paragraphs have office hours. But then these can be added to a bundle of Geo entities.

@finnlewis finnlewis marked this pull request as draft August 27, 2024 11:17
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