From 95b139d7569bc328bb0a34adde34e653ec06c604 Mon Sep 17 00:00:00 2001
From: jdlrobson <jdlrobson@gmail.com>
Date: Thu, 7 Feb 2019 12:02:18 -0800
Subject: [PATCH] page actions is no longer position absolute

This was causing lots of problems with the AMC design and seems
unnecessary. It dates back to tablet support added in
I14c8182473c9508ffc38a8d5baf114bcb016a35f in 2014. This no longer
appears to be needed for tablet.

Changing the HTML means we need to be careful about Varnish cached
HTML with new styles. To avoid UI regressions we use a new temporary
class heading-holding--new which will be removed as soon as possible.
The no-page-actions modifier is no longer needed given the new heading
holder doesn't care what's inside it.

Bug: T212216
Change-Id: I7e989a3d4553eb3357598a5cad3ccebf51dc9fae
---
 includes/skins/minerva.mustache               |  4 +--
 .../pageactions.less                          | 25 ++++++++++++++++---
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/includes/skins/minerva.mustache b/includes/skins/minerva.mustache
index 67db140..df57f82 100644
--- a/includes/skins/minerva.mustache
+++ b/includes/skins/minerva.mustache
@@ -28,12 +28,12 @@
 		<main id="content" class="mw-body">
 			{{#hasheadingholder}}
 			{{{prebodyhtml}}}
-			<div class="pre-content heading-holder {{^haspageactions}}heading-holder--no-page-actions{{/haspageactions}}">
-				{{{pageactionshtml}}}
+			<div class="pre-content heading-holder heading-holder--new">
 				{{{headinghtml}}}
 				{{{taglinehtml}}}
 				{{{postheadinghtml}}}
 				{{{subtitle}}}
+				{{{pageactionshtml}}}
 				{{{internalBanner}}}
 			</div>
 			{{/hasheadingholder}}
diff --git a/resources/skins.minerva.base.styles/pageactions.less b/resources/skins.minerva.base.styles/pageactions.less
index f2f323f..1ed5b3b 100644
--- a/resources/skins.minerva.base.styles/pageactions.less
+++ b/resources/skins.minerva.base.styles/pageactions.less
@@ -4,17 +4,30 @@
 .heading-holder {
 	@pageActionsGutter: 0.5em;
 	@pageActionsHeight: @pageActionFontSize + (2 * @iconGutterWidth);
+	// FIXME: padding-bottom can be removed when work on T212216
+	// has been deployed and varnish cache cleared.
 	padding: @titleSectionSpacingTop 0 (@pageActionsHeight + @pageActionsGutter);
 	position: relative;
 
+	// FIXME: Can be made default when work on T212216
+	// has been deployed and varnish cache cleared.
+	&--new {
+		padding-bottom: 0;
+	}
+	// FIXME: Can be removed when work on T212216
+	// has been deployed and varnish cache cleared.
 	&--no-page-actions {
 		padding-bottom: @pageActionsGutter;
+
+		&--new {
+			padding-bottom: 0;
+		}
 	}
 
 	.tagline {
 		color: @colorGray5;
 		font-size: 0.85em;
-		margin: 2px 0 0;
+		margin: 2px 0 12px;
 	}
 }
 
@@ -29,9 +42,8 @@
 	float: none;
 	border: 0;
 	overflow: hidden;
-	position: absolute;
-	bottom: 0;
 	width: 100%;
+	margin-top: -2px;
 	border-top: 1px solid @colorGray14;
 	border-bottom: 1px solid @colorGray12;
 	padding: 0.5em 0;
@@ -78,6 +90,13 @@
 	}
 }
 
+// FIXME: cached HTML. Can be removed when work on T212216
+// has been deployed and varnish cache cleared.
+.heading-holder #page-actions:first-child {
+	position: absolute;
+	bottom: 0;
+}
+
 // Watchstar is hidden for anonymous no-js users
 // While we could link the icon to the login/signup form, this is not
 // a perfect experience and could be confusing.