From eb1f1e28a31e6013f2add8fdcab5407ea1d5e132 Mon Sep 17 00:00:00 2001
From: Thiemo Kreuz <thiemo.kreuz@wikimedia.de>
Date: Tue, 13 Jul 2021 12:15:47 +0200
Subject: [PATCH] Make new template editor sidebar items actual ButtonElements

Actually reusing this OOUI mixin gives us a lot of well
developed functionality we need anyway. Most notably proper
event management, e.g. click events.

The number of CSS properties we need to override is managable,
I would argue. Let's see:
* Our buttons are not inline-elements, but should use the full
  width.
* No focus-border left and right for the same reason.
* We want much more inner padding.
* We want a stronger hover effect.
* We need to fine-tune the position of the icon. This is
  because of the inner padding.
* Need to get rid of a negative margin that's only relevant
  for inline-buttons.

I currently feel like the benefits are worth living with
slightly more brittle code. Note that we can undo this change
any time because all this is well encapsulated in this new
class.

Bug: T274544
Change-Id: I33f275a958964d49e803e56bf74a6fa961093da1
---
 .../dialogs/ve.ui.MWTransclusionDialog.css    | 43 +++++++++++--------
 ...ve.ui.MWTransclusionOutlineButtonWidget.js | 12 +++++-
 .../ve.ui.MWTransclusionOutlinePartWidget.js  |  2 +-
 3 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/modules/ve-mw/ui/styles/dialogs/ve.ui.MWTransclusionDialog.css b/modules/ve-mw/ui/styles/dialogs/ve.ui.MWTransclusionDialog.css
index d85e3dbdd6..5225017bf0 100644
--- a/modules/ve-mw/ui/styles/dialogs/ve.ui.MWTransclusionDialog.css
+++ b/modules/ve-mw/ui/styles/dialogs/ve.ui.MWTransclusionDialog.css
@@ -68,23 +68,6 @@
 	outline: 1px solid #0f0;
 }
 
-.ve-ui-mwTransclusionOutlineButtonWidget {
-	cursor: pointer;
-	font-weight: bold;
-	overflow: hidden;
-	padding: 11px 0 11px 37px;
-	position: relative;
-	text-overflow: ellipsis;
-}
-
-.ve-ui-mwTransclusionOutlineButtonWidget:hover {
-	background-color: #eaecf0;
-}
-
-.ve-ui-mwTransclusionOutlineButtonWidget .oo-ui-iconElement-icon {
-	left: 11px;
-}
-
 .ve-ui-mwTemplateDialog .oo-ui-outlineOptionWidget.oo-ui-flaggedElement-empty .oo-ui-iconElement-icon {
 	opacity: 0.51;
 }
@@ -159,3 +142,29 @@
 .ve-ui-mwTransclusionOutlineItem .oo-ui-checkboxInputWidget.oo-ui-widget-enabled [ type='checkbox' ]:hover + span {
 	border-width: 2px;
 }
+
+.ve-ui-mwTransclusionOutlineButtonWidget,
+.ve-ui-mwTransclusionOutlineButtonWidget > .oo-ui-buttonElement-button {
+	display: block;
+}
+
+.ve-ui-mwTransclusionOutlineButtonWidget.oo-ui-buttonElement-frameless.oo-ui-labelElement.oo-ui-iconElement > .oo-ui-buttonElement-button {
+	border-left: 0;
+	border-radius: 0;
+	border-right: 0;
+	overflow: hidden;
+	padding: 8px 0 8px 36px;
+	text-overflow: ellipsis;
+}
+
+.ve-ui-mwTransclusionOutlineButtonWidget.oo-ui-buttonElement-frameless.oo-ui-widget-enabled > .oo-ui-buttonElement-button:hover {
+	background-color: #eaecf0;
+}
+
+.ve-ui-mwTransclusionOutlineButtonWidget.oo-ui-buttonElement-frameless.oo-ui-iconElement:first-child {
+	margin-left: 0;
+}
+
+.ve-ui-mwTransclusionOutlineButtonWidget.oo-ui-buttonElement-frameless.oo-ui-iconElement > .oo-ui-buttonElement-button > .oo-ui-iconElement-icon {
+	left: 10px;
+}
diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineButtonWidget.js b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineButtonWidget.js
index 91970f691e..f73dd60533 100644
--- a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineButtonWidget.js
+++ b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineButtonWidget.js
@@ -1,4 +1,7 @@
 /**
+ * Generic button-like widget for items in the template dialog sidebar. See {OO.ui.ButtonWidget} for
+ * inspiration.
+ *
  * @class
  * @extends OO.ui.Widget
  *
@@ -12,18 +15,23 @@ ve.ui.MWTransclusionOutlineButtonWidget = function VeUiMWTransclusionOutlineButt
 	ve.ui.MWTransclusionOutlineButtonWidget.super.call( this, config );
 
 	// Mixin constructors
+	OO.ui.mixin.ButtonElement.call( this, {
+		framed: false
+	} );
 	OO.ui.mixin.IconElement.call( this, config );
 	OO.ui.mixin.LabelElement.call( this, config );
 
 	this.$element
 		.addClass( 've-ui-mwTransclusionOutlineButtonWidget' )
-		.prepend( this.$icon, this.$label );
+		.append( this.$button.append( this.$icon, this.$label ) );
 };
 
 /* Inheritance */
 
 OO.inheritClass( ve.ui.MWTransclusionOutlineButtonWidget, OO.ui.Widget );
+OO.mixinClass( ve.ui.MWTransclusionOutlineButtonWidget, OO.ui.mixin.ButtonElement );
 OO.mixinClass( ve.ui.MWTransclusionOutlineButtonWidget, OO.ui.mixin.IconElement );
 OO.mixinClass( ve.ui.MWTransclusionOutlineButtonWidget, OO.ui.mixin.LabelElement );
-// TODO: Add OO.ui.mixin.AccessKeyedElement?
 // TODO: Add OO.ui.mixin.TitledElement?
+// TODO: Add OO.ui.mixin.TabIndexedElement?
+// TODO: Add OO.ui.mixin.AccessKeyedElement?
diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlinePartWidget.js b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlinePartWidget.js
index 8d7c78971e..1d474da087 100644
--- a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlinePartWidget.js
+++ b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlinePartWidget.js
@@ -30,7 +30,7 @@ ve.ui.MWTransclusionOutlinePartWidget = function VeUiMWTransclusionOutlinePartWi
 		.addClass( 've-ui-mwTransclusionOutlinePartWidget' )
 		// Note: There is no code that uses this. It just helps when manually inspecting the HTML.
 		.attr( 'data-transclusion-part-id', part.getId() )
-		.prepend( header.$element );
+		.append( header.$element );
 };
 
 /* Inheritance */