Support more (all) vCard 4 properties

Created on 6 October 2023, 12 months ago
Updated 2 April 2024, 6 months ago

Problem/Motivation

This module is still missing quite a lot of the properties that the vCard format (https://datatracker.ietf.org/doc/html/rfc6350#section-6) supports. It would be nice to offer a way that would support all properties without having to hardcode them all in the views plugins and templates.

Proposed resolution

Convert the individual properties to plugins that can have their own forms and rendered output. This makes it more dynamic and easier to add properties later.

I think the feeds module uses a similar model.

Remaining tasks

  • Create a plugin annotation
  • Create a base class
  • Create plugins for the currently supported types
  • Update the views style class
  • Remove the views row class(?)
  • Update template
  • Database update

User interface changes

Instead of presenting all properties, have a dynamic table where the desired properties can be selected and then associate views fields to them.

API changes

Change the properties to be plugins instead. This allows them to come with their own subforms for assigning fields (to sub-properties if applicable) and render the property's line.

Data model changes

All individual template fields would be removed and rendering would happen in each property plugin instead. More properties can be added either in this module or in a custom module (this would also allow ovverriding the module's default output). Template should go back the the bare minimum and print BEGIN, END en VERSION.

✨ Feature request
Status

Needs work

Version

3.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States chrisolof

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @chrisolof
  • Status changed to Closed: won't fix 12 months ago
  • πŸ‡³πŸ‡±Netherlands Neograph734 Netherlands

    Hi, thanks for the feedback. However, creating UI fields for all custom properties feels a bit overkill. There is also no support for the SOUND property, to name something.

    The UI is designed to work for most use-cases. If you require other properties, I suppose you are better off overriding the module's template file (views-vcards-view-row-vcard.html.twig) in your own theme.
    You can either misuse one of the other fields, or use a hook_preprocess_HOOK to add a new variable to the list.

  • πŸ‡ΊπŸ‡ΈUnited States chrisolof

    Oh bummer. I was incorrectly assuming this module was open to complete v4.0 property support.

    At any rate I've already created a patch to add NOTE property support, so I'll post it here for your consideration. Maybe it will help others with similar needs.

    Thank you for the work-around advice.

  • πŸ‡ΊπŸ‡ΈUnited States chrisolof

    New patch attached, created against the latest 3.x branch code.

  • Status changed to Active 12 months ago
  • πŸ‡³πŸ‡±Netherlands Neograph734 Netherlands

    Oh bummer. I was incorrectly assuming this module was open to complete v4.0 property support.

    Don't get me wrong, I am not against it. However, I don't think it would be compatible with the way this module is currently built. There would be endless lists of properties everywhere and people with overridden templates would not be able to see the newly added fields.

    I suppose that the better way would be to overhaul the whole field assignment form. Instead of putting all the properties there, use a drop-down to the select the vcard property and then assign fields (to sub-properties if applicable). But then I suppose that all properties would have to be created as annontated plugins with their own form and rendered output. It would be quite the overhaul...

    I'll try to have a look the coming days, but cannot promise that I will ever finish it...

  • Status changed to Needs review 12 months ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update 12 months ago
    2 pass
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update 12 months ago
    2 pass
  • πŸ‡³πŸ‡±Netherlands Neograph734 Netherlands

    @chrisolof, I don't know why the commit messages don't show up here, but I have been playing in the issue fork. If you have some time, please give it a try. Adding the NOTE property is now a matter of copying one of the provided classes and making minimal changes.

    Be aware that there is no upgrade path yet, so please try the module in a development environment.

  • πŸ‡³πŸ‡±Netherlands Neograph734 Netherlands
  • πŸ‡ΊπŸ‡ΈUnited States chrisolof

    Oh this is very cool! I'll try and get you a review here next week. I really like the new plugin design.

  • Status changed to Needs work 11 months ago
  • πŸ‡ΊπŸ‡ΈUnited States chrisolof

    This is really neat. The flexibility to add multiple types of each property (say a work and home phone number, for example) and configure them all independently per your unique requirements immediately struck me as a nice improvement over the previous, fairly hard-coded model.

    I haven't done a full implementation test against this branch yet, but after some light testing and code review I spotted a few potential improvements we might want to make at the early stages here:

    - Omit rendering of empty property lines. I believe the prior setup accomplished as much via conditional output in the template. I just pushed this change into the issue fork.
    - I suspect the render() method in the Photo plugin is not necessary, given the render method in VcardPropertyPluginBase.
    - I wonder if supportsHomeWorkType might be better held in the annotation config / plugin definition...
    - It seems to me that the plugin interface could drop prepareOutputString(), as it seems to be more of just a helper-method in VcardPropertyPluginBase, with nothing outside of VcardPropertyPluginBase depending on it. The render method seems like the one to keep in the interface since it's what the rest of the system hooks up to for output from the plugin. It's possible a plugin could decide to render by other means (or more directly), without following the prepareOutputString() then render() model.
    - Document return values on prepareOutputString() and render(). I took a stab at this in my push to the issue fork since the empty-checking was related.

    Again, I haven't done extensive testing, but the basics appear to be in place and working here. I would say this still needs work, especially given the lack of an upgrade path, and it probably deserves a major version change when it lands in a release, but it is definitely all coming together here.

  • πŸ‡³πŸ‡±Netherlands Neograph734 Netherlands

    Thanks for taking the time to have a look!

    - Omit rendering of empty property lines. I believe the prior setup accomplished as much via conditional output in the template. I just pushed this change into the issue fork.

    This makes sense. Thanks.

    - I suspect the render() method in the Photo plugin is not necessary, given the render method in VcardPropertyPluginBase.

    The render method for the photo plugin is different because it wraps base64 encoded data at 64 characters instead of 75 which is the limit for other vCard properties per https://datatracker.ietf.org/doc/html/rfc6350#section-3.2.

    - I wonder if supportsHomeWorkType might be better held in the annotation config / plugin definition...

    I suppose I was a bit biased by how Views does this for display/row/style plugins. But it sure makes sense to move it.

    - It seems to me that the plugin interface could drop prepareOutputString(), as it seems to be more of just a helper-method in VcardPropertyPluginBase, with nothing outside of VcardPropertyPluginBase depending on it. The render method seems like the one to keep in the interface since it's what the rest of the system hooks up to for output from the plugin. It's possible a plugin could decide to render by other means (or more directly), without following the prepareOutputString() then render() model.

    I was definatly thinking of a new major relase, because the compatibility with the old template (and possible overrides people made there). The upgrade path is not impossible to write. It is just a long list of old properties to read and the new format to put them into. That only requires time...

  • Merge request !2Switch to plugin based vCard properties β†’ (Merged) created by Neograph734
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update 6 months ago
    2 fail
Production build 0.71.5 2024