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.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | views_vcards-add-note-support-3392249-13.patch | 4.79 KB | vaish |
Issue fork views_vcards-3392249
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
neograph734Hi, 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.
Comment #3
chrisolofOh 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.
Comment #4
chrisolofNew patch attached, created against the latest 3.x branch code.
Comment #5
neograph734Don'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...
Comment #6
neograph734@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.
Comment #7
neograph734Comment #8
chrisolofOh this is very cool! I'll try and get you a review here next week. I really like the new plugin design.
Comment #9
chrisolofThis 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.
Comment #10
neograph734Thanks for taking the time to have a look!
This makes sense. Thanks.
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 suppose I was a bit biased by how Views does this for display/row/style plugins. But it sure makes sense to move it.
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...
Comment #12
vaish commentedPatch from #4 re-rolled for the 3.1.x version. Great work on the version 4 of the module!
Comment #13
vaish commentedPatch from #12 re-rolled to work with version 3.1.1.