[2024-11-07] TinyMCE: Insert contact, but track using linkcheck #3169

PeterNerlich · 2024-11-01T17:56:02Z

Short description

We want to embed contacts on pages and in other content. This PR is an alternative to #3124.
Contact cards contain a hidden link, allowing us to track where a contact is used with the linkchecker.

Proposed changes

Add a URL to each contact, making it trackable by linkcheck
Add a custom TinyMCE plugin to embed contacts via a button and dialog, similar to our custom link plugin
In the contact form, show where contacts are being embedded
saving changes to a contact will update all translation objects referencing the contact

Side effects

linkcheck will check the contact links and find that no resource is being served at those URIs. THey are shown as valid, but we could also hide them.
The URI contains the region slug, but it is never relevant in the code for now

Known issues

Adding a contact to a page/poi/event is not immediately reflected in the contact form. It is reflected eventually, once the linkchecker has run again. (You can manually trigger this with ./tools/integreat-cms-cli findlinks, although even that is not 100% reliable...)
This is not ideal; IDK if we could trigger a linkcheck run every time a contact is saved...? Or can a Link manually be inserted into the DB?

Open quetions

The shortcut for adding a "[K]ontakt" is Ctrl+L, the shortcut for adding a "[L]ink" is Ctrl+K...
That's not exactly intuitive. @osmers is it feasible to switch these two, even though users might already be used to inserting links with Ctrl+K?

Resolved issues

Fixes: #2959

Pull Request Review Guidelines

PeterNerlich · 2024-11-07T04:24:14Z

Adding TomSelect could easily be its own PR, but we already have so many of those. Feel free to split this up if you find a sensible way how

charludo · 2024-12-02T09:18:56Z

This is now ready for review @PeterNerlich @david-venhoff @JoeyStk @MizukiTemma

MizukiTemma · 2024-12-03T11:47:04Z

I haven't yet finished reviewing but following functions seem to be behaving good:

No contact of other regions appears in the search result
No archived contact appears in the search result
String match in the search works
A contact can be embedded in a single content/multiple of contents
Multiple of contacts can be embedded in a single content
A contact can be embedded multiple of times in a single content
Changing an embedded contact to another works, the card is updated
Updating details of an embedded contact updates the card(s) in the content(s)
Embedded contact cards can be removed
Referencing contents are correctly recognised and shown in the contact form
DeepL, SUMM.AI and Google Translate can be used as before (for translation of contact cards see Translate point_of_contact_for content in contact cards #3259)

We probably need either protection of embedded contacts from deleting/archiving or automatic deletion of contact cards at the moment of contact deletion. An issue is opened for this topic #3260

MizukiTemma · 2024-12-03T13:46:48Z

Thank you so much for tackling this complicated issue 💪 This PR looks already super cool, the main functions seem to be working as expected 🥺 😻

I have some suggestions to the contact card.

We may add the name of POI if the contact has neither point_of_contact_for nor name.
Address of POI probably makes the contact card more informative (especially) if the contact has none of E-mail, phone number and website.
No : if no name is provided? or we can probably look for an alternative, as : looks (in my opinion) a bit strange after : for gender-neutral position name.

@osmers Your opinion is important here 👂

ah, maybe a part of/related to #3258 ?

osmers · 2024-12-04T07:13:04Z

I'd like to be able to test this myself to be able to say exactly what is missing, but from the things I can see here:

We definitely need to include the address in a contact, if it's provided
Do we already have the option now to select which contact details I want added to the contact card?
Has the HIX problem been resolved? From the Mattermost thread I understand that it has been, right?

Everything else is a bit hard to decide without trying the function for myself and so far I don't see it on test yet, right?

Switching shortcuts - difficult question. I think it might confuse the people who are already using the existing shortcut, especially since it's the same for any other program to add a link. I think I would keep it, even though it's not intuitive... maybe @nikolahoff has some insight into user behaviour here as well?

charludo · 2024-12-04T08:12:27Z

We may add the name of POI if the contact has neither point_of_contact_for nor name.

Do you mean that if the contact has no name, the user can enter one? If so, I think that is really difficult to achieve right now - it's also a bit contrary to the idea that users should manage contaacts in a central location. Or do yo mean that user can update existing contacts with a name on-the-fly and it gets saved to the contact model?
Sorry if I am misunderstanding you here 🙈

Address of POI probably makes the contact card more informative (especially) if the contact has none of E-mail, phone number and website.

👍🏼 Good idea! Will add.

No : if no name is provided? or we can probably look for an alternative, as : looks (in my opinion) a bit strange after : for gender-neutral position name.

Could we just drop the ":" altogether? Then it doesn't matter if a name is provided, and can't inferfer with ":" in titles. I think the bold font is probably enough separation? Not sure though.

charludo · 2024-12-04T08:27:07Z

I'd like to be able to test this myself to be able to say exactly what is missing

If you have a couple of minutes after the weekly, I could screenshare? Not the same as testing yourself, I know, but perhaps better than just reading about it 😄

We definitely need to include the address in a contact, if it's provided

Will add. In the Grooming yesterday, opening hours were also mentioned - though I suspect that is something for down-the-line?

Do we already have the option now to select which contact details I want added to the contact card?

No, this will be added after this PR is merged: #3258

Has the HIX problem been resolved? From the Mattermost thread I understand that it has been, right?

No, not yet - just opened #3268 for this, thanks for the reminder 😅

Everything else is a bit hard to decide without trying the function for myself and so far I don't see it on test yet, right?

Yep, that'll only happen after this is merged.

Switching shortcuts - difficult question. I think it might confuse the people who are already using the existing shortcut, especially since it's the same for any other program to add a link. I think I would keep it, even though it's not intuitive... maybe @nikolahoff has some insight into user behaviour here as well?

Ah, fair point, I thought Ctrl+K for Links was "our invention", so to speak (I only knew it as "search globally" 😄).
But I am not sure if Ctrl_L is the best choice for contacts then. Maybe Ctrl+P for "Person"?

osmers · 2024-12-04T08:53:32Z

@charludo that would be great - though I'm only free at 2pm for half an hour 😅

Regarding the shortcut - in word Ctrl+K is also for links :D and in Thunderbird as well...
Ctrl+P is known for printing, right? When I use it right now my browser wants to print the page...

Opening hours were initially planned by us but then dropped and the municipalities mentioned that they would need it bcs the opening hours for a contact can be quite different from the POI (which we had planned to just take). But I guess for the time being they could add them manually to the editor and we can implement it as an update afterwards...

charludo · 2024-12-04T09:24:59Z

Regarding the shortcut - in word Ctrl+K is also for links :D and in Thunderbird as well... Ctrl+P is known for printing, right? When I use it right now my browser wants to print the page...

Ctrl+P would be overriden by our own shortcut. IMO that's fine, I doubt anyone needs to print the CMS editor view... 😅

bcs the opening hours for a contact can be quite different from the POI

OK, so this is a planned feature, to be able to add "contact opening hours", and after tha't implemented we can add them here?

But I guess for the time being they could add them manually to the editor and we can implement it as an update afterwards...

Not 1005 sure what you mean - do you mean they can just add them as plain text in the editor? If so, sure, that's fine (albeit some extra work for them when we introduce the feature).
But they can not edit the contact card itself by changing text or adding fields.

MizukiTemma · 2024-12-04T09:36:17Z

@charludo

We may add the name of POI if the contact has neither point_of_contact_for nor name.

Do you mean that if the contact has no name, the user can enter one? If so, I think that is really difficult to achieve right now - it's also a bit contrary to the idea that users should manage contaacts in a central location. Or do yo mean that user can update existing contacts with a name on-the-fly and it gets saved to the contact model?
Sorry if I am misunderstanding you here 🙈

No no, I didn't mean that complicated 🙈
Each contact has a POI, and my idea is to display the name of this POI in the card in the position of point_of_contact_for and name.

charludo · 2024-12-04T09:37:33Z

@charludo

We may add the name of POI if the contact has neither point_of_contact_for nor name.

Do you mean that if the contact has no name, the user can enter one? If so, I think that is really difficult to achieve right now - it's also a bit contrary to the idea that users should manage contaacts in a central location. Or do yo mean that user can update existing contacts with a name on-the-fly and it gets saved to the contact model?
Sorry if I am misunderstanding you here 🙈

No no, I didn't mean that complicated 🙈 Each contact has a POI, and my idea is to display the name of this POI in the card in the position of point_of_contact_for and name.

Ohhhhhh that makes sense. Sorry 😄
Will add!

charludo · 2024-12-07T10:23:17Z

@MizukiTemma how about this for avoiding the ":"?

Also, if I understood @osmers correctly, the POI name should not be shown afterall, only its address.

@osmers is the styling OK with you? Reduced the gaps as suggested. No gaps look kinda weird however.

MizukiTemma

Thank you so much 🥺 🙏 Looks good 🚀

charludo · 2024-12-10T07:14:17Z

I've added a link to the Integreat Map, if it is shown there, to Google Maps otherwise. I've also updated the link shown in the contact edit view to use the same, consistent link to the Integreat Map.

david-venhoff

Thank you so much for this massive pr, it looks really cool 🚀

I have a bunch of suggestions, but most of them are relatively minor or just nits.

integreat_cms/cms/templates/contacts/contact_card.html

integreat_cms/cms/models/contact/contact.py

integreat_cms/cms/utils/internal_link_checker.py

integreat_cms/cms/views/utils/contact_utils.py

integreat_cms/static/src/css/tinymce_custom.css

integreat_cms/static/src/js/tinymce-plugins/custom_contact_input/plugin.js

charludo · 2024-12-12T10:44:59Z

I have a bunch of suggestions, but most of them are relatively minor or just nits.

Thank you for the thorough review!! I think I was able to address most of it, apart from the unresolved conversation above.

integreat_cms/cms/templates/contacts/contact_card.html

david-venhoff

Awesome, let's give it a try 🥳

integreat_cms/cms/templates/contacts/contact_card.html

integreat_cms/static/src/css/tinymce_custom.css

integreat_cms/static/src/css/contact_card.css

integreat_cms/static/src/css/tinymce_custom.css

MizukiTemma · 2024-12-17T18:33:06Z

@charludo
Don't forget to squash the commits before merge 😸

charludo · 2024-12-18T06:54:54Z

@david-venhoff fantastic, thank you, those changes all make perfect sense 👍🏼
@MizukiTemma 👀 👀 👀 👀

PeterNerlich requested review from charludo and david-venhoff November 1, 2024 17:56

PeterNerlich force-pushed the feature/tinymce-insert-contact-use-linkcheck branch from 03928ef to 9e1ff12 Compare November 1, 2024 18:44

charludo self-assigned this Nov 2, 2024

PeterNerlich force-pushed the feature/tinymce-insert-contact-use-linkcheck branch from 11f8851 to 36b8c05 Compare November 7, 2024 05:27

charludo mentioned this pull request Nov 29, 2024

Add styling for contact cards digitalfabrik/integreat-app#3010

Open

charludo force-pushed the feature/tinymce-insert-contact-use-linkcheck branch from af2ce21 to 5e76e83 Compare December 2, 2024 09:17

charludo marked this pull request as ready for review December 2, 2024 09:18

charludo requested review from JoeyStk and MizukiTemma December 2, 2024 09:19

MizukiTemma added prio: high Needs to be resolved ASAP. deadline Needs to be fixed in the given time labels Dec 2, 2024

This was referenced Dec 3, 2024

Add tests for TinyMCE Contact insertion & updates #3257

Open

Allow users to choose what contact details are shown when inserting contacts in TinyMCE #3258

Open

MizukiTemma mentioned this pull request Dec 3, 2024

Prevent orphaned contact cards #3260

Open

charludo force-pushed the feature/tinymce-insert-contact-use-linkcheck branch from 49e066b to fa0f8e1 Compare December 7, 2024 10:25

MizukiTemma approved these changes Dec 9, 2024

View reviewed changes

charludo mentioned this pull request Dec 9, 2024

Register a new URL and Link in Linkcheck whenever a contact is inserted in TinyMCE #3280

Open

1 task

charludo mentioned this pull request Dec 9, 2024

Exclude contact cards (and other non-editable blocks) from HIX #3281

Open

MizukiTemma added the blocker This issue blocks another issue label Dec 9, 2024

charludo force-pushed the feature/tinymce-insert-contact-use-linkcheck branch from 3a4f17e to bebba4b Compare December 11, 2024 09:25

MizukiTemma added prio: urgent Needs to be resolved now(?) and removed prio: high Needs to be resolved ASAP. labels Dec 11, 2024

david-venhoff requested changes Dec 11, 2024

View reviewed changes

charludo force-pushed the feature/tinymce-insert-contact-use-linkcheck branch from 85877d0 to 1e51ce7 Compare December 12, 2024 10:44

charludo requested a review from david-venhoff December 12, 2024 10:45

david-venhoff reviewed Dec 12, 2024

View reviewed changes

integreat_cms/cms/templates/contacts/contact_card.html Outdated Show resolved Hide resolved

charludo force-pushed the feature/tinymce-insert-contact-use-linkcheck branch from 1e51ce7 to 29a989f Compare December 16, 2024 08:32

charludo requested a review from david-venhoff December 16, 2024 08:32

david-venhoff approved these changes Dec 17, 2024

View reviewed changes

Feature: Insert contact cards into TinyMCE

0944716

charludo force-pushed the feature/tinymce-insert-contact-use-linkcheck branch from 81a9c96 to 0944716 Compare December 18, 2024 07:01

charludo merged commit 2e01833 into develop Dec 18, 2024
5 checks passed

charludo deleted the feature/tinymce-insert-contact-use-linkcheck branch December 18, 2024 07:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2024-11-07] TinyMCE: Insert contact, but track using linkcheck #3169

[2024-11-07] TinyMCE: Insert contact, but track using linkcheck #3169

PeterNerlich commented Nov 1, 2024 •

edited by charludo

Loading

PeterNerlich commented Nov 7, 2024

charludo commented Dec 2, 2024

MizukiTemma commented Dec 3, 2024 •

edited

Loading

MizukiTemma commented Dec 3, 2024 •

edited

Loading

osmers commented Dec 4, 2024

charludo commented Dec 4, 2024 •

edited

Loading

charludo commented Dec 4, 2024

osmers commented Dec 4, 2024

charludo commented Dec 4, 2024

MizukiTemma commented Dec 4, 2024

charludo commented Dec 4, 2024

charludo commented Dec 7, 2024

MizukiTemma left a comment

charludo commented Dec 10, 2024

david-venhoff left a comment •

edited

Loading

charludo commented Dec 12, 2024

david-venhoff left a comment

MizukiTemma commented Dec 17, 2024

charludo commented Dec 18, 2024

[2024-11-07] TinyMCE: Insert contact, but track using linkcheck #3169

[2024-11-07] TinyMCE: Insert contact, but track using linkcheck #3169

Conversation

PeterNerlich commented Nov 1, 2024 • edited by charludo Loading

Short description

Proposed changes

Side effects

Known issues

Open quetions

Resolved issues

PeterNerlich commented Nov 7, 2024

charludo commented Dec 2, 2024

MizukiTemma commented Dec 3, 2024 • edited Loading

MizukiTemma commented Dec 3, 2024 • edited Loading

osmers commented Dec 4, 2024

charludo commented Dec 4, 2024 • edited Loading

charludo commented Dec 4, 2024

osmers commented Dec 4, 2024

charludo commented Dec 4, 2024

MizukiTemma commented Dec 4, 2024

charludo commented Dec 4, 2024

charludo commented Dec 7, 2024

MizukiTemma left a comment

Choose a reason for hiding this comment

charludo commented Dec 10, 2024

david-venhoff left a comment • edited Loading

Choose a reason for hiding this comment

charludo commented Dec 12, 2024

david-venhoff left a comment

Choose a reason for hiding this comment

MizukiTemma commented Dec 17, 2024

charludo commented Dec 18, 2024

PeterNerlich commented Nov 1, 2024 •

edited by charludo

Loading

MizukiTemma commented Dec 3, 2024 •

edited

Loading

MizukiTemma commented Dec 3, 2024 •

edited

Loading

charludo commented Dec 4, 2024 •

edited

Loading

david-venhoff left a comment •

edited

Loading