From f9a8703a64d12fcd57607245fcf62db9b71232ce Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Fri, 5 Nov 2021 11:21:09 -0700 Subject: [PATCH] Minerva uses core page title handling Bug: T265892 Change-Id: I2ffdedd64414ffb3c6e441391a75fd6e744847a4 --- i18n/en.json | 1 - i18n/qqq.json | 1 - includes/Skins/SkinMinerva.php | 57 ++++++++----------- includes/Skins/skin.mustache | 4 +- .../pageactions.less | 3 + .../skins.minerva.mainPage.styles/common.less | 3 + .../page-issues/page/pageIssueFormatter.js | 4 +- .../mediawiki.action.edit.styles/minerva.less | 5 +- skinStyles/mobile.special.styles/minerva.less | 2 +- .../features/support/pages/article_page.js | 2 +- tests/selenium/pageobjects/edit.page.js | 2 +- 11 files changed, 41 insertions(+), 43 deletions(-) diff --git a/i18n/en.json b/i18n/en.json index 001ea903f..e4900c3fe 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -15,7 +15,6 @@ "mobile-frontend-home-button": "Home", "mobile-frontend-language-article-heading": "Language", "mobile-frontend-languages-not-available": "This page is not available in other languages.", - "mobile-frontend-logged-in-homepage-notification": "{{GENDER:$1|Welcome}}, $1!", "mobile-frontend-main-menu-button-tooltip": "Open main menu", "mobile-frontend-main-menu-contributions": "Contributions", "mobile-frontend-main-menu-login": "Log in", diff --git a/i18n/qqq.json b/i18n/qqq.json index ab6400976..5b0aec6dc 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -29,7 +29,6 @@ "mobile-frontend-home-button": "This appears in the mobile web menu, which appears when tapping the three stripes at the top corner.\n\nThis button takes the user to the home page of the wiki site.\n{{Identical|Home}}", "mobile-frontend-language-article-heading": "In the secondary page menu, a short label that describes the button that displays a list of available languages\n{{Identical|Language}}", "mobile-frontend-languages-not-available": "A toast message that is shown when the user taps on the language button on an article which is not available in other languages.", - "mobile-frontend-logged-in-homepage-notification": "Heading at top of a wiki's main page telling user they are logged in. Parameters:\n* $1 - username\n{{Identical|Welcome}}", "mobile-frontend-main-menu-button-tooltip": "Tooltip for menu button", "mobile-frontend-main-menu-contributions": "Contributions link in main menu\n{{Identical|Contribution}}", "mobile-frontend-main-menu-login": "Text for log in link in main menu.\n{{Identical|Log in}}", diff --git a/includes/Skins/SkinMinerva.php b/includes/Skins/SkinMinerva.php index 170c82ccd..38e4f1e30 100644 --- a/includes/Skins/SkinMinerva.php +++ b/includes/Skins/SkinMinerva.php @@ -115,12 +115,19 @@ class SkinMinerva extends SkinMustache { unset( $data['html-categories'] ); } + // Special handling for certain pages. + // This is technical debt that should be upstreamed to core. + $isUserPage = $this->getUserPageHelper()->isUserPage(); + $isUserPageAccessible = $this->getUserPageHelper()->isUserPageAccessibleToCurrentUser(); + if ( $isUserPage && $isUserPageAccessible ) { + $data['html-title-heading'] = $this->getUserPageHeadingHtml( $data['html-title-heading' ] ); + } + return $data + [ 'array-minerva-banners' => $this->prepareBanners( $data['html-site-notice'] ), 'html-minerva-user-notifications' => $this->prepareUserNotificationsButton( $this->getNewtalks() ), 'data-minerva-main-menu' => $this->getMainMenu()->getMenuData()['items'], 'html-minerva-tagline' => $this->getTaglineHtml(), - 'html-minerva-heading' => $this->prepareHeader(), 'html-minerva-post-heading' => $this->isTalkPageWithViewAction() ? $this->getTalkPagePostHeadingHtml() : '', @@ -537,18 +544,23 @@ class SkinMinerva extends SkinMustache { /** * Returns the HTML representing the heading. + * + * @param string $heading The heading suggested by core. * @return string HTML for header */ - protected function getHeadingHtml() { - $isUserPage = $this->getUserPageHelper()->isUserPage(); - $isUserPageAccessible = $this->getUserPageHelper()->isUserPageAccessibleToCurrentUser(); - if ( $isUserPage && $isUserPageAccessible ) { - // The heading is just the username without namespace - $heading = $this->getUserPageHelper()->getPageUser()->getName(); - } else { - $heading = $this->getOutput()->getPageTitle(); - } - return Html::rawElement( 'h1', [ 'id' => 'section_0' ], $heading ); + private function getUserPageHeadingHtml( $heading ) { + // The heading is just the username without namespace + // This is escaped as a precaution (user name should be safe). + return Html::rawElement( 'h1', + // These IDs and classes should match Skin::getTemplateData + [ + 'id' => 'firstHeading', + 'class' => 'firstHeading mw-first-heading mw-minerva-user-heading', + ], + htmlspecialchars( + $this->getUserPageHelper()->getPageUser()->getName() + ) + ); } /** @@ -625,29 +637,6 @@ class SkinMinerva extends SkinMustache { return $html; } - /** - * Create and prepare header content - * @return string - */ - protected function prepareHeader() { - $title = $this->getTitle(); - if ( $title->isMainPage() ) { - $user = $this->getUser(); - $msg = $this->msg( 'mobile-frontend-logged-in-homepage-notification', $user->getName() ); - - if ( $user->isRegistered() && !$msg->isDisabled() ) { - $out = $this->getOutput(); - $out->setPageTitle( $msg->text() ); - } else { - // Don’t add any

to main pages - // for logged-out users - return ''; - } - } - - return $this->getHeadingHtml(); - } - /** * Load internal banner content to show in pre content in template * Beware of HTML caching when using this function. diff --git a/includes/Skins/skin.mustache b/includes/Skins/skin.mustache index 1b75ca89d..c0db73a54 100644 --- a/includes/Skins/skin.mustache +++ b/includes/Skins/skin.mustache @@ -56,9 +56,10 @@ {{{html-user-message}}} {{/html-user-message}} + {{^is-title-blank}}
- {{{html-minerva-heading}}} + {{{html-title-heading}}} {{{html-minerva-tagline}}}
{{#data-minerva-tabs}} @@ -74,6 +75,7 @@ {{{html-minerva-post-heading}}}
{{{html-subtitle}}}
+ {{/is-title-blank}}
{{{html-body-content}}} {{! This shows "Return to page" on talk page when talk tab not shown at top. diff --git a/resources/skins.minerva.base.styles/pageactions.less b/resources/skins.minerva.base.styles/pageactions.less index b0d63e0ba..f050e7523 100644 --- a/resources/skins.minerva.base.styles/pageactions.less +++ b/resources/skins.minerva.base.styles/pageactions.less @@ -86,6 +86,9 @@ display: none; } + // FIXME: #section_0 rule can be removed > 1 week after + // I2ffdedd64414ffb3c6e441391a75fd6e744847a4 is in production. + .mw-first-heading, #section_0 { // stylelint-disable-line selector-max-id border: 0; } diff --git a/resources/skins.minerva.mainPage.styles/common.less b/resources/skins.minerva.mainPage.styles/common.less index 5efcc9f46..2aab7710b 100644 --- a/resources/skins.minerva.mainPage.styles/common.less +++ b/resources/skins.minerva.mainPage.styles/common.less @@ -3,6 +3,9 @@ // stylelint-disable selector-max-id .page-Main_Page { + // FIXME: #section_0 rule can be removed > 1 week after + // I2ffdedd64414ffb3c6e441391a75fd6e744847a4 is in production. + .mw-first-heading, #section_0 { border: 0; } diff --git a/resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js b/resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js index 5125d7471..83f9bcbf4 100644 --- a/resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js +++ b/resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js @@ -42,8 +42,10 @@ function insertPageIssueNotice( labelText, section ) { var $link = newPageIssueLink( labelText ); $link.attr( 'href', '#/issues/' + section ); + // FIXME: #section_0 rule can be removed > 1 week after + // I2ffdedd64414ffb3c6e441391a75fd6e744847a4 is in production. // eslint-disable-next-line no-jquery/no-global-selector - $link.insertAfter( $( 'h1#section_0' ) ); + $link.insertAfter( $( 'h1#section_0, h1.mw-first-heading' ) ); } module.exports = { diff --git a/skinStyles/mediawiki.action.edit.styles/minerva.less b/skinStyles/mediawiki.action.edit.styles/minerva.less index a8340045c..5865c0977 100644 --- a/skinStyles/mediawiki.action.edit.styles/minerva.less +++ b/skinStyles/mediawiki.action.edit.styles/minerva.less @@ -40,13 +40,14 @@ // Parsing information doesn't need to be so big // neither to headers for diffs (.diff-otitle), - // the page title (#section_0) or all the warnings (#editpage-copywarn, #mw-anon-edit-warning) + // the page title (.mw-first-heading) + // or all the warnings (#editpage-copywarn, #mw-anon-edit-warning) .mw-editnotice, .mw-editTools, .preview-limit-report-wrapper, .diff-otitle, .diff-ntitle, - #section_0, + .mw-first-heading, #editpage-copywarn, #mw-anon-edit-warning { .secondary-text(); diff --git a/skinStyles/mobile.special.styles/minerva.less b/skinStyles/mobile.special.styles/minerva.less index 0f9a2d9ae..ee01472e1 100644 --- a/skinStyles/mobile.special.styles/minerva.less +++ b/skinStyles/mobile.special.styles/minerva.less @@ -42,7 +42,7 @@ } .ns-special { - .mw-body #section_0 { + .mw-body .mw-first-heading { font-size: 1.5em; font-weight: bold; } diff --git a/tests/selenium/features/support/pages/article_page.js b/tests/selenium/features/support/pages/article_page.js index e088cccc0..949343e2f 100644 --- a/tests/selenium/features/support/pages/article_page.js +++ b/tests/selenium/features/support/pages/article_page.js @@ -33,7 +33,7 @@ class ArticlePage extends MinervaPage { get notifications_button_element() { return $( '#pt-notifications-alert' ); } get drawer_element() { return $( '.drawer' ); } get edit_link_element() { return $( '#ca-edit' ); } - get first_heading_element() { return $( '#section_0' ); } + get first_heading_element() { return $( 'h1.mw-first-heading' ); } get notification_element() { return $( '.mw-notification-area .mw-notification' ); } get overlay_heading_element() { return $( '.overlay-title h2' ); } get overlay_category_topic_item_element() { return $( '.topic-title-list li' ); } diff --git a/tests/selenium/pageobjects/edit.page.js b/tests/selenium/pageobjects/edit.page.js index 20e72085a..0f404e482 100644 --- a/tests/selenium/pageobjects/edit.page.js +++ b/tests/selenium/pageobjects/edit.page.js @@ -5,7 +5,7 @@ const Page = require( 'wdio-mediawiki/Page' ); class EditPage extends Page { get content() { return $( '#wikitext-editor' ); } get displayedContent() { return $( '#mw-content-text .mw-parser-output' ); } - get heading() { return $( '#section_0' ); } + get heading() { return $( 'h1.mw-first-heading' ); } get next() { return $( '.mw-ui-icon-mf-next-invert' ); } get save() { return $( 'button.mw-ui-button' ); }