diff: Add legend and tooltips to inline diff display
Add a legend at the top of the inline diff display, showing the meanings of the colours of the inserted and deleted highlighting. Also add the same text as tooltips on the highlighted elements. The legend is added as part of a new area above the diff table that can be modified via a new TextSlotDiffRendererTablePrefix hook, so that extensions can add other buttons etc. there as required. This is a follow-up to the previous attempt, which added the legend in DifferenceEngine::showDiff() and was called from too many places. This patch moves it to be called in DifferenceEngine::showDiffPage(). Bug: T324759 Change-Id: I2a3c67bcfa47313dee597e602a62073e4e298cd2 Follow-up: I6de30bf79eb5ac262285951792782b870d075e00
This commit is contained in:
parent
6a3e33f330
commit
1eb586013c
11 changed files with 193 additions and 6 deletions
|
|
@ -136,6 +136,13 @@ For notes on 1.39.x and older releases, see HISTORY.
|
|||
MW_SKIP_EXTERNAL_DEPENDENCIES=1 environment flag when running PHPUnit.
|
||||
* ManualLogEntry::setForceBotFlag() has been added to allow the forcing of the
|
||||
bot flag for log entries which are inserted to the recent changes.
|
||||
* A new hook, TextSlotDiffRendererTablePrefixHook, has been added to allow
|
||||
extensions to add content within #mw-content-text but after the
|
||||
DifferenceEngineViewHeader or DifferenceEngineShowDiffPage hooks have been
|
||||
run. The new hook is used to add elements within a horizontal display area,
|
||||
where their order can be explicitly set. Examples of uses for this include
|
||||
adding the VisualEditor diff-type switch, and the legend for inline diffs that
|
||||
is displayed if Wikidiff2 is installed.
|
||||
|
||||
=== External library changes in 1.40 ===
|
||||
|
||||
|
|
|
|||
|
|
@ -1054,6 +1054,7 @@ $wgAutoloadLocalClasses = [
|
|||
'MediaWiki\\Diff\\Hook\\DifferenceEngineShowEmptyOldContentHook' => __DIR__ . '/includes/diff/Hook/DifferenceEngineShowEmptyOldContentHook.php',
|
||||
'MediaWiki\\Diff\\Hook\\DifferenceEngineViewHeaderHook' => __DIR__ . '/includes/diff/Hook/DifferenceEngineViewHeaderHook.php',
|
||||
'MediaWiki\\Diff\\Hook\\NewDifferenceEngineHook' => __DIR__ . '/includes/diff/Hook/NewDifferenceEngineHook.php',
|
||||
'MediaWiki\\Diff\\Hook\\TextSlotDiffRendererTablePrefixHook' => __DIR__ . '/includes/diff/Hook/TextSlotDiffRendererTablePrefixHook.php',
|
||||
'MediaWiki\\Diff\\WordAccumulator' => __DIR__ . '/includes/diff/WordAccumulator.php',
|
||||
'MediaWiki\\EditPage\\Constraint\\AccidentalRecreationConstraint' => __DIR__ . '/includes/editpage/Constraint/AccidentalRecreationConstraint.php',
|
||||
'MediaWiki\\EditPage\\Constraint\\AutoSummaryMissingSummaryConstraint' => __DIR__ . '/includes/editpage/Constraint/AutoSummaryMissingSummaryConstraint.php',
|
||||
|
|
|
|||
|
|
@ -83,6 +83,7 @@ class HookRunner implements
|
|||
\MediaWiki\Diff\Hook\AbortDiffCacheHook,
|
||||
\MediaWiki\Diff\Hook\ArticleContentOnDiffHook,
|
||||
\MediaWiki\Diff\Hook\DifferenceEngineAfterLoadNewTextHook,
|
||||
\MediaWiki\Diff\Hook\TextSlotDiffRendererTablePrefixHook,
|
||||
\MediaWiki\Diff\Hook\DifferenceEngineLoadTextAfterNewContentIsLoadedHook,
|
||||
\MediaWiki\Diff\Hook\DifferenceEngineMarkPatrolledLinkHook,
|
||||
\MediaWiki\Diff\Hook\DifferenceEngineMarkPatrolledRCIDHook,
|
||||
|
|
@ -1322,6 +1323,17 @@ class HookRunner implements
|
|||
);
|
||||
}
|
||||
|
||||
public function onTextSlotDiffRendererTablePrefix(
|
||||
\TextSlotDiffRenderer $textSlotDiffRenderer,
|
||||
IContextSource $context,
|
||||
array &$parts
|
||||
) {
|
||||
return $this->container->run(
|
||||
'TextSlotDiffRendererTablePrefix',
|
||||
[ $textSlotDiffRenderer, $context, &$parts ]
|
||||
);
|
||||
}
|
||||
|
||||
public function onDifferenceEngineLoadTextAfterNewContentIsLoaded(
|
||||
$differenceEngine
|
||||
) {
|
||||
|
|
|
|||
|
|
@ -679,6 +679,8 @@ abstract class ContentHandler {
|
|||
$slotDiffRenderer->setStatsdDataFactory( $statsdDataFactory );
|
||||
// XXX using the page language would be better, but it's unclear how that should be injected
|
||||
$slotDiffRenderer->setLanguage( $contentLanguage );
|
||||
$slotDiffRenderer->setHookContainer( MediaWikiServices::getInstance()->getHookContainer() );
|
||||
$slotDiffRenderer->setContentModel( $this->getModelID() );
|
||||
|
||||
$inline = ( $options['diff-type'] ?? '' ) === 'inline';
|
||||
$engine = DifferenceEngine::getEngine();
|
||||
|
|
|
|||
|
|
@ -919,6 +919,11 @@ class DifferenceEngine extends ContextSource {
|
|||
$notice .= Html::warningBox( $msg->parse() );
|
||||
}
|
||||
|
||||
// Add table prefixes.
|
||||
foreach ( $this->getSlotDiffRenderers() as $slotDiffRenderer ) {
|
||||
$out->addHTML( $slotDiffRenderer->getTablePrefix( $this->getContext() ) );
|
||||
}
|
||||
|
||||
$this->showDiff( $oldHeader, $newHeader, $notice );
|
||||
if ( !$diffOnly ) {
|
||||
$this->renderNewRevision();
|
||||
|
|
@ -1615,13 +1620,14 @@ class DifferenceEngine extends ContextSource {
|
|||
}
|
||||
|
||||
/**
|
||||
* Add title attributes for tooltips on moved paragraph indicators
|
||||
* Add title attributes for tooltips on various diff elements
|
||||
*
|
||||
* @param string $text
|
||||
* @return string
|
||||
*/
|
||||
private function addLocalisedTitleTooltips( $text ) {
|
||||
return preg_replace_callback(
|
||||
// Moved paragraph indicators.
|
||||
$text = preg_replace_callback(
|
||||
'/class="mw-diff-movedpara-(left|right)"/',
|
||||
function ( array $matches ) {
|
||||
$key = $matches[1] === 'right' ?
|
||||
|
|
@ -1631,6 +1637,20 @@ class DifferenceEngine extends ContextSource {
|
|||
},
|
||||
$text
|
||||
);
|
||||
|
||||
// For inline diffs, add tooltips to `<ins>` and `<del>`.
|
||||
if ( isset( $this->slotDiffOptions['diff-type'] ) && $this->slotDiffOptions['diff-type'] == 'inline' ) {
|
||||
$text = str_replace(
|
||||
[ '<ins>', '<del>' ],
|
||||
[
|
||||
Html::openElement( 'ins', [ 'title' => $this->msg( 'diff-inline-tooltip-ins' )->plain() ] ),
|
||||
Html::openElement( 'del', [ 'title' => $this->msg( 'diff-inline-tooltip-del' )->plain() ] ),
|
||||
],
|
||||
$text
|
||||
);
|
||||
}
|
||||
|
||||
return $text;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
33
includes/diff/Hook/TextSlotDiffRendererTablePrefixHook.php
Normal file
33
includes/diff/Hook/TextSlotDiffRendererTablePrefixHook.php
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
<?php
|
||||
|
||||
namespace MediaWiki\Diff\Hook;
|
||||
|
||||
use IContextSource;
|
||||
use TextSlotDiffRenderer;
|
||||
|
||||
/**
|
||||
* This is a hook handler interface, see docs/Hooks.md.
|
||||
* Use the hook name "TextSlotDiffRendererTablePrefix" to register handlers implementing this interface.
|
||||
*
|
||||
* @stable to implement
|
||||
* @ingroup Hooks
|
||||
*/
|
||||
interface TextSlotDiffRendererTablePrefixHook {
|
||||
|
||||
/**
|
||||
* Use this hook to change the HTML that is included in a prefix container directly before the diff table.
|
||||
*
|
||||
* @since 1.41
|
||||
*
|
||||
* @param TextSlotDiffRenderer $textSlotDiffRenderer
|
||||
* @param IContextSource $context
|
||||
* @param mixed[] &$parts HTML strings to add to a container above the diff table.
|
||||
* Will be sorted by key before being output.
|
||||
* @return bool|void True or no return value to continue or false to abort
|
||||
*/
|
||||
public function onTextSlotDiffRendererTablePrefix(
|
||||
TextSlotDiffRenderer $textSlotDiffRenderer,
|
||||
IContextSource $context,
|
||||
array &$parts
|
||||
);
|
||||
}
|
||||
|
|
@ -49,6 +49,16 @@ abstract class SlotDiffRenderer {
|
|||
*/
|
||||
abstract public function getDiff( Content $oldContent = null, Content $newContent = null );
|
||||
|
||||
/**
|
||||
* Get the content to add above the main diff table.
|
||||
*
|
||||
* @param IContextSource $context
|
||||
* @return string The full HTML for the prefix area, with its contents.
|
||||
*/
|
||||
public function getTablePrefix( IContextSource $context ): string {
|
||||
return '';
|
||||
}
|
||||
|
||||
/**
|
||||
* Add modules needed for correct styling/behavior of the diff.
|
||||
* @stable to override
|
||||
|
|
|
|||
|
|
@ -21,6 +21,9 @@
|
|||
* @ingroup DifferenceEngine
|
||||
*/
|
||||
|
||||
use MediaWiki\HookContainer\HookContainer;
|
||||
use MediaWiki\HookContainer\HookRunner;
|
||||
use MediaWiki\Html\Html;
|
||||
use MediaWiki\MediaWikiServices;
|
||||
use MediaWiki\Shell\Shell;
|
||||
use Wikimedia\Assert\Assert;
|
||||
|
|
@ -49,18 +52,26 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
|
|||
/** Use an external executable. */
|
||||
public const ENGINE_EXTERNAL = 'external';
|
||||
|
||||
public const INLINE_LEGEND_KEY = '10_mw-diff-inline-legend';
|
||||
|
||||
/** @var IBufferingStatsdDataFactory|null */
|
||||
private $statsdDataFactory;
|
||||
|
||||
/** @var Language|null The language this content is in. */
|
||||
private $language;
|
||||
|
||||
/** @var HookRunner|null */
|
||||
private $hookRunner;
|
||||
|
||||
/** @var string One of the ENGINE_* constants. */
|
||||
private $engine = self::ENGINE_PHP;
|
||||
|
||||
/** @var string|null Path to an executable to be used as the diff engine. */
|
||||
private $externalEngine;
|
||||
|
||||
/** @var string */
|
||||
private $contentModel;
|
||||
|
||||
/** @inheritDoc */
|
||||
public function getExtraCacheKeys() {
|
||||
// Tell DifferenceEngine this is a different variant from the standard wikidiff2 variant
|
||||
|
|
@ -99,6 +110,22 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
|
|||
$this->language = $language;
|
||||
}
|
||||
|
||||
/**
|
||||
* @since 1.41
|
||||
* @param HookContainer $hookContainer
|
||||
*/
|
||||
public function setHookContainer( HookContainer $hookContainer ): void {
|
||||
$this->hookRunner = new HookRunner( $hookContainer );
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $contentModel
|
||||
* @since 1.41
|
||||
*/
|
||||
public function setContentModel( string $contentModel ) {
|
||||
$this->contentModel = $contentModel;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set which diff engine to use.
|
||||
* @param string $type One of the ENGINE_* constants.
|
||||
|
|
@ -120,6 +147,16 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
|
|||
$this->externalEngine = $executable;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the content model ID that this renderer acts on
|
||||
*
|
||||
* @since 1.41
|
||||
* @return string
|
||||
*/
|
||||
public function getContentModel(): string {
|
||||
return $this->contentModel;
|
||||
}
|
||||
|
||||
/** @inheritDoc */
|
||||
public function getDiff( Content $oldContent = null, Content $newContent = null ) {
|
||||
$this->normalizeContents( $oldContent, $newContent, TextContent::class );
|
||||
|
|
@ -130,6 +167,41 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
|
|||
return $this->getTextDiff( $oldText, $newText );
|
||||
}
|
||||
|
||||
/**
|
||||
* @inheritDoc
|
||||
*/
|
||||
public function getTablePrefix( IContextSource $context ): string {
|
||||
$legend = null;
|
||||
// wikidiff2 inline type gets a legend to explain the highlighting colours.
|
||||
if ( $this->engine === self::ENGINE_WIKIDIFF2_INLINE ) {
|
||||
$ins = Html::element(
|
||||
'span', [ 'class' => 'mw-diff-inline-legend-ins' ], $context->msg( 'diff-inline-tooltip-ins' )->plain()
|
||||
);
|
||||
$del = Html::element(
|
||||
'span', [ 'class' => 'mw-diff-inline-legend-del' ], $context->msg( 'diff-inline-tooltip-del' )->plain()
|
||||
);
|
||||
$legend = Html::rawElement( 'div', [ 'class' => 'mw-diff-inline-legend' ], "$ins $del" );
|
||||
}
|
||||
// Allow extensions to add other parts to this area (or modify the legend).
|
||||
// An empty placeholder for the legend is added when it's not in use and other items have been added.
|
||||
$parts = [ self::INLINE_LEGEND_KEY => $legend ];
|
||||
|
||||
$this->hookRunner->onTextSlotDiffRendererTablePrefix( $this, $context, $parts );
|
||||
if ( count( $parts ) > 1 && $parts[self::INLINE_LEGEND_KEY] === null ) {
|
||||
$parts[self::INLINE_LEGEND_KEY] = Html::element( 'div' );
|
||||
}
|
||||
ksort( $parts );
|
||||
if ( count( array_filter( $parts ) ) > 0 ) {
|
||||
$attrs = [
|
||||
'class' => 'mw-diff-table-prefix',
|
||||
'dir' => $this->language->getDir(),
|
||||
'lang' => $this->language->getCode(),
|
||||
];
|
||||
return Html::rawElement( 'div', $attrs, implode( '', $parts ) );
|
||||
}
|
||||
return '';
|
||||
}
|
||||
|
||||
/**
|
||||
* Diff the text representations of two content objects (or just two pieces of text in general).
|
||||
* @param string $oldText
|
||||
|
|
|
|||
|
|
@ -1014,6 +1014,8 @@
|
|||
"diff-multi-manyusers": "({{PLURAL:$1|One intermediate revision|$1 intermediate revisions}} by more than $2 {{PLURAL:$2|user|users}} not shown)",
|
||||
"diff-paragraph-moved-tonew": "Paragraph was moved. Click to jump to new location.",
|
||||
"diff-paragraph-moved-toold": "Paragraph was moved. Click to jump to old location.",
|
||||
"diff-inline-tooltip-ins": "Content added",
|
||||
"diff-inline-tooltip-del": "Content deleted",
|
||||
"difference-missing-revision": "{{PLURAL:$2|One revision|$2 revisions}} of this difference ($1) {{PLURAL:$2|was|were}} not found.\n\nThis is usually caused by following an outdated diff link to a page that has been deleted.\nDetails can be found in the [{{fullurl:{{#Special:Log}}/delete|page={{FULLPAGENAMEE}}}} deletion log].",
|
||||
"difference-bad-old-revision": "The content of the old revision is missing or corrupted.",
|
||||
"difference-bad-new-revision": "The content of the new revision is missing or corrupted.",
|
||||
|
|
|
|||
|
|
@ -1265,6 +1265,8 @@
|
|||
"diff-multi-manyusers": "This message appears in the revision history of a page when comparing two versions which aren't consecutive, and the intermediate revisions have been edited by more than 100 users.\n\nParameters:\n* $1 - the number of revisions, will always be 101 or more\n* $2 - the number of users that were found, which was limited at 100\n{{Related|Diff-multi}}",
|
||||
"diff-paragraph-moved-tonew": "Explaining title tag for the indicating symbols when a paragraph was moved hinting to the new location in the Diff view.",
|
||||
"diff-paragraph-moved-toold": "Explaining title tag for the indicating symbols when a paragraph was moved hinting to the old location in the Diff view.",
|
||||
"diff-inline-tooltip-ins": "Title for inserted content in a diff, shown as a tooltip on the content and also in a legend at the top of the diff.",
|
||||
"diff-inline-tooltip-del": "Title for deleted content in a diff, shown as a tooltip on the content and also in a legend at the top of the diff.",
|
||||
"difference-missing-revision": "Text displayed when the requested revision does not exist using a diff link.\n\nExample: [{{canonicalurl:Project:News|diff=426850&oldid=99999999}} Diff with invalid revision#]\n\nParameters:\n* $1 - the list of missing revision IDs\n* $2 - the number of items in $1 (one or two)",
|
||||
"difference-bad-old-revision": "Text displayed when the old (left-hand side) revision of a diff cannot be loaded due to database corruption.",
|
||||
"difference-bad-new-revision": "Text displayed when the new (right-hand side) revision of a diff cannot be loaded due to database corruption.",
|
||||
|
|
|
|||
|
|
@ -2,6 +2,9 @@
|
|||
* Diff rendering
|
||||
*/
|
||||
|
||||
@diffInserted: #a3d3ff;
|
||||
@diffDeleted: #ffe49c;
|
||||
|
||||
/* stylelint-disable selector-class-pattern */
|
||||
|
||||
.diff {
|
||||
|
|
@ -74,12 +77,35 @@
|
|||
|
||||
.mw-diff-inline-added ins,
|
||||
.mw-diff-inline-changed ins {
|
||||
background: #a3d3ff;
|
||||
background: @diffInserted;
|
||||
}
|
||||
|
||||
.mw-diff-inline-deleted del,
|
||||
.mw-diff-inline-changed del {
|
||||
background: #ffe49c;
|
||||
background: @diffDeleted;
|
||||
}
|
||||
|
||||
.mw-diff-table-prefix {
|
||||
display: flex;
|
||||
justify-content: space-between;
|
||||
align-items: center;
|
||||
margin: 1em 0;
|
||||
|
||||
.mw-diff-inline-legend {
|
||||
.mw-diff-inline-legend-ins,
|
||||
.mw-diff-inline-legend-del {
|
||||
display: inline-block;
|
||||
padding: 4px 6px;
|
||||
}
|
||||
|
||||
.mw-diff-inline-legend-ins {
|
||||
background: @diffInserted;
|
||||
}
|
||||
|
||||
.mw-diff-inline-legend-del {
|
||||
background: @diffDeleted;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
.diff-addedline,
|
||||
|
|
@ -125,11 +151,11 @@
|
|||
}
|
||||
|
||||
.diff-addedline {
|
||||
border-color: #a3d3ff;
|
||||
border-color: @diffInserted;
|
||||
}
|
||||
|
||||
.diff-deletedline {
|
||||
border-color: #ffe49c;
|
||||
border-color: @diffDeleted;
|
||||
}
|
||||
|
||||
.diffchange {
|
||||
|
|
|
|||
Loading…
Reference in a new issue