I'm looking to use this module on my site but am using the BEM methodology for CSS name classes. Since the HTML/CSS is hard-coded into the theme.inc file, it's really difficult to remove or override classes or add other HTML attributes. I've created this patch to move the class declarations to a preprocess function so that they can be modified and overridden as needed by a user. It uses the drupal_attributes function which allows for any needed attributes to be declared for each element.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

SeeWatson created an issue. See original summary.

SeeWatson’s picture

SeeWatson’s picture

Not sure if $vars['settings'] is the best place for these since that consists of actual settings from the display formatter. I can move them to $vars['attributes'] or something similar if that would make more sense.

johnv’s picture

Happy with your contribution. In the issue queue are some complaints about the inline css.

SeeWatson’s picture

Updated to use $vars['attributes'] to store values to keep them separate from $vars['settings'] that holds actual UI settings.

SeeWatson’s picture

Actually, I'm going to go even further here and remove the theme function entirely and replace it with a render array generated in the preprocess function and a template file. That will give themers full control of the output and will align better with best practices. I'll do some work on that and probably have a patch by mid-week.

johnv’s picture

Are you working on the D7-version. Is it still worth it? It seems more efficient/rewarding to work on the D8-version.

SeeWatson’s picture

Patch is attached. It's a fairly significant rework of how the default field formatter and the theming layer interact. It will allow for near complete freedom in theming the HTML and CSS through adding preprocess functions and overriding the template files. Assuming I didn't make any mistakes, it should also, by default, render identically to how the current module renders, so it will be backwards compatible. Since I changed the theme render names, a cache clear will be required for fields to display after updating. That happens automatically when update.php is run, so I don't think that will be a big issue.

The only people who may have an issue with the update are those who overrode the theme function in a custom module or theme to get custom output. They will see the field revert back to default styles. Overriding the theme function is a really terrible practice, so while I wish it wouldn't do that to them, I don't see another way around it.

As to D8, I'm not currently running any D8 sites, but most of the D8 theming conventions are just more strongly enforced versions of the D7 best practices. I actually took a lot of inspiration (and code) from the D7 and D8 paragraphs module and the core D8 image module. Everything I've done here should transfer seamlessly to D8 with a reworking of the template files to use twig. I may have some time to put into that in the next month. Let me know if there are any issues or changes needed, and hopefully others will test it and it will work well.

SeeWatson’s picture

Fixed some incorrect documentation.

spf-lpd’s picture

Can this be applied to D8 at the moment?
If so, can we please get instructions?

Thanks!

SeeWatson’s picture

I'm working on a D8 patch, but it will likely be another week or two before I can get it finalized.

SeeWatson’s picture

I'd like to get some eyes on the D7 version to make sure it's all working as expected before I commit a lot of work to porting this to D8.

johnv’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev

@SeeWatson, given the explanation you give about impiicatio on existing installations, I prefer to implement this improvement only in D8. The D7-sites will mostly be out of the project phase, and adding an extra development task to this installations will mmot profit this modules reputation.

For D8, I think this is a good improvement, since we are in the early phase of this module's D8 life cycle.

johnv’s picture

Component: Code » Code - formatter
johnv’s picture

Version: 8.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Closed (won't fix)
Related issues: +#2849551: Cleanup formatter and use twig

I am sorry for your work SeeWatson, but in D7 I am not going to add this anymore.

For D8, we have done improvements in #2849551: Cleanup formatter and use twig

Thanks for your contribution.