Problem/Motivation

The 8.x-1.x branch still supports Drupal 9 and old versions of PHP.

We should cut a new 2.x branch requiring:
- Drupal 10.1
- PHP 8.1

This will allow us to do things like constructor property promotion, stricter typing, etc.

This could also be a chance to significantly refactor portions of the module that are outdated.

Issue fork diff-3440375

Command icon 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:

About issue forks

Comments

acbramley created an issue. See original summary.

dww’s picture

dww
we/he/they
commented

Sounds great. +1

acbramley’s picture

acbramley commented
Version: 8.x-1.x-dev » 2.x-dev
luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented

Great to see some movement in this module. I invite ya'll to check out https://drupal.org/project/diff_plus for anything that might be beneficial to include in a new major.

acbramley opened merge request !51

acbramley’s picture

acbramley commented

@Luke.Leber very keen to collaborate there! If there's anything you can point out that may be good candidates I'm all ears :)

mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
commented

Not sure if you want to include D10 deprecations in the scope of this issue, but this module does use entity_storage->loadRevisions(), which is removed in Drupal 11, per https://www.drupal.org/node/3294237

Located in the following files:
- /src/Controller/NodeRevisionController.php
- /src/Controller/PluginRevisionController.php
- /src/Form/RevisionOverviewForm.php
- /src/Plugin/views/field/DiffPluginBase.php

Update: there may not be any real work involved in this transition: loadRevision() should automatically come from the new location in Drupal core.

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented

Some feature suggestions for 2.x:

A more extensible twig template based UI.

Presently, the UI can be rather spartan and cold compared to the implementations in other content management systems. Something as simple as user avatars can make a world of difference to non-technical users. Giving themes the ability to customize how the Diff UI looks is a sorely needed UX enhancement. This is how I heavy-handedly tackled this in diff_plus: https://git.drupalcode.org/project/diff_plus/-/blob/1.0.x/src/EventSubsc...

The default HTMLPurifier configuration just doesn't work very well sometimes.

I ditched HTMLPurifier in diff_plus and added some tweaks and toggles to make higher fidelity HTML5 complex content renderings ( https://git.drupalcode.org/project/diff_plus/-/blob/1.0.x/src/Plugin/dif... ).

There has never been a more opportune moment to make Diff entity agnostic.

Drupal isn't all about nodes anymore. We have all sorts of entity types in contrib and custom-land, now with a generic revision UI available, a major version bump seems like the right time to target getting all modernized here. This has not been tackled in diff_plus yet.

If these are too extreme, I'll gladly continue maintaining things in diff_plus, but just wanted to weigh in and let the maintainers know that I'm very open to collaboration.

Thanks.

acbramley’s picture

acbramley commented

@Luke.Leber great points, fully agree with the generic entity stuff, I just wish Node was using that already so it was even easier.

I am keen to explore other HTML diffing libraries, when I last checked the one this module uses is quite outdated?

acbramley’s picture

acbramley commented
Status: Active » Fixed

This is done, the last thing I want to do before cutting an alpha is #3449233: Investigate better HTML diffing libraries to replace HTMLPurifier

Making the module entity agnostic will come once Node uses the generic revision UI :) (in a 3.x branch perhaps)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.