Move the warning outside sitenotice, tidy up code

There's a lot of things going on here, so a quick summary:
* Moved the warning outside sitenotice to avoid reflows and all
  kinds of other issues, including those with dismissable notices.
* Placed the warning on the bottom of the page, on all skins, for
  consistency and to avoid obstructing vital UI elements such as
  site name, user tools, search box.
* Changed the UI to use OOUI for, again, consistency and to avoid
  reinventing the wheel with button styling.
* Removed the cookie emoji as it was only taking up valuable space
  on mobile and making designing a sensible layout much harder.
* Merged the mobile and desktop RL modules into a single, responsive
  one. The warning will work correctly for any skin and screen width.
* The integration tests are truly horrible, I tried to at least
  make them work. They deserve a proper rewrite, but that should be
  done after the extension switches to the new hook system.

I will post screenshots of this in action in the first of the linked
tasks.

Bug: T271047
Bug: T173335
Change-Id: I6e2a3d7aeccc0d4df1b3238e52c67e72099d27d8
This commit is contained in:
Ostrzyciel 2021-02-01 10:54:49 +01:00
parent 1f5075224e
commit edd0a7d949
8 changed files with 127 additions and 218 deletions

View file

@ -1,6 +1,3 @@
{
"extends": "stylelint-config-wikimedia",
"rules": {
"rule-empty-line-before": null
}
"extends": "stylelint-config-wikimedia"
}

View file

@ -1,10 +1,11 @@
{
"name": "CookieWarning",
"version": "0.2.0",
"version": "0.3.0",
"author": [
"Florian Schmidt",
"Liz Lee",
"Jack Phoenix"
"Jack Phoenix",
"Ostrzyciel"
],
"url": "https://www.mediawiki.org/wiki/Extension:CookieWarning",
"descriptionmsg": "cookiewarning-desc",
@ -19,10 +20,10 @@
]
},
"Hooks": {
"SiteNoticeAfter": "CookieWarning\\Hooks::onSiteNoticeAfter",
"BeforePageDisplay": "CookieWarning\\Hooks::onBeforePageDisplay",
"SkinAfterContent": "CookieWarning\\Hooks::onSkinAfterContent",
"GetPreferences": "CookieWarning\\Hooks::onGetPreferences",
"BeforeInitialize": "CookieWarning\\Hooks::onBeforeInitialize",
"BeforePageDisplay": "CookieWarning\\Hooks::onBeforePageDisplay",
"ResourceLoaderGetConfigVars": "CookieWarning\\Hooks::onResourceLoaderGetConfigVars"
},
"config": {
@ -94,15 +95,10 @@
]
},
"ext.CookieWarning.styles": {
"styles": "resources/ext.CookieWarning/ext.CookieWarning.less"
},
"ext.CookieWarning.mobile.styles": {
"styles": "resources/ext.CookieWarning/ext.CookieWarning.mobile.less",
"skinStyles": {
"minerva": "skinStyles/ext.CookieWarning.minerva.styles.less"
},
"styles": "resources/ext.CookieWarning/ext.CookieWarning.less",
"targets": [
"mobile"
"mobile",
"desktop"
]
},
"ext.CookieWarning.geolocation": {

View file

@ -4,12 +4,13 @@ namespace CookieWarning;
use Config;
use ConfigException;
use ExtensionRegistry;
use Html;
use MediaWiki;
use MediaWiki\MediaWikiServices;
use MobileContext;
use MWException;
use OOUI\ButtonInputWidget;
use OOUI\ButtonWidget;
use OOUI\HorizontalLayout;
use OutputPage;
use Skin;
use Title;
@ -38,7 +39,7 @@ class Hooks {
return;
}
if ( $user->isLoggedIn() ) {
if ( $user->isRegistered() ) {
$user->setOption( 'cookiewarning_dismissed', 1 );
$user->saveSettings();
} else {
@ -48,20 +49,16 @@ class Hooks {
}
/**
* SiteNoticeAfter hook handler.
* SkinAfterContent hook handler.
*
* Adds the CookieWarning information bar to the output html for mobile.
* Adds the CookieWarning information bar to the output html.
*
* @param string &$siteNotice
* @param string &$data
* @param Skin $skin
* @throws ConfigException
*
* @throws MWException
* @return bool|void True or no return value to continue or false to abort
*/
public static function onSiteNoticeAfter(
string &$siteNotice,
Skin $skin
) {
public static function onSkinAfterContent( string &$data, Skin $skin ) {
/** @var Decisions $cookieWarningDecisions */
$cookieWarningDecisions = MediaWikiServices::getInstance()
->getService( 'CookieWarning.Decisions' );
@ -70,64 +67,53 @@ class Hooks {
return;
}
$isMobile = ExtensionRegistry::getInstance()->isLoaded( 'MobileFrontend' ) &&
MobileContext::singleton()->shouldDisplayMobileView();
$siteNotice .= self::generateElements( $skin, $isMobile );
$data .= self::generateElements( $skin );
}
/**
* Generates the elements for the banner.
*
* @param Skin $skin
* @param bool $isMobile This will return true if using mobile site.
* @return string|null The html for cookie notice.
*/
private static function generateElements( Skin $skin, bool $isMobile ) {
private static function generateElements( Skin $skin ) : ?string {
$moreLink = self::getMoreLink();
$buttons = [];
if ( $moreLink ) {
$moreLink = "\u{00A0}" . Html::element(
'a',
[ 'href' => $moreLink ],
$skin->msg( 'cookiewarning-moreinfo-label' )->text()
);
$buttons[] = new ButtonWidget( [
'href' => $moreLink,
'label' => $skin->msg( 'cookiewarning-moreinfo-label' )->text(),
'flags' => [ 'progressive' ]
] );
}
$form = Html::openElement( 'form', [ 'method' => 'POST' ] ) .
Html::submitButton(
$skin->msg( 'cookiewarning-ok-label' )->text(),
[
$buttons[] = new ButtonInputWidget( [
'type' => 'submit',
'label' => $skin->msg( 'cookiewarning-ok-label' )->text(),
'name' => 'disablecookiewarning',
'class' => 'mw-cookiewarning-dismiss'
]
) .
Html::closeElement( 'form' );
'value' => 'OK',
'flags' => [ 'primary', 'progressive' ]
] );
$cookieImage = Html::element(
'div',
[ 'class' => 'mw-cookiewarning-cimage' ],
"\u{1F36A}"
$form = Html::rawElement(
'form',
[ 'method' => 'POST' ],
new HorizontalLayout( [ 'items' => $buttons ] )
);
return Html::openElement(
'div',
// banner-container marks this as a banner for Minerva
// Note to avoid this class, in future we may want to make use of SiteNotice
// or banner display
[ 'class' => 'mw-cookiewarning-container banner-container' ]
[ 'class' => 'mw-cookiewarning-container' ]
) .
Html::openElement(
'div',
[ 'class' => 'mw-cookiewarning-text' ]
) .
( $isMobile ? $cookieImage : '' ) .
Html::element(
'span',
[],
$skin->msg( 'cookiewarning-info' )->text()
) .
$moreLink .
Html::closeElement( 'div' ) .
$form .
Html::closeElement( 'div' );
@ -143,7 +129,7 @@ class Hooks {
* @return string|null The url or null if none set
* @throws ConfigException
*/
private static function getMoreLink() {
private static function getMoreLink() : ?string {
$conf = self::getConfig();
if ( $conf->get( 'CookieWarningMoreUrl' ) ) {
return $conf->get( 'CookieWarningMoreUrl' );
@ -180,15 +166,8 @@ class Hooks {
return;
}
if (
ExtensionRegistry::getInstance()->isLoaded( 'MobileFrontend' ) &&
MobileContext::singleton()->shouldDisplayMobileView()
) {
$moduleStyles = [ 'ext.CookieWarning.mobile.styles' ];
} else {
$moduleStyles = [ 'ext.CookieWarning.styles' ];
}
$modules = [ 'ext.CookieWarning' ];
$moduleStyles = [ 'ext.CookieWarning.styles' ];
if ( $cookieWarningDecisions->shouldAddResourceLoaderComponents() ) {
$modules[] = 'ext.CookieWarning.geolocation';
@ -196,6 +175,7 @@ class Hooks {
}
$out->addModules( $modules );
$out->addModuleStyles( $moduleStyles );
$out->enableOOUI();
}
/**

View file

@ -13,7 +13,7 @@
$( '.mw-cookiewarning-container' ).detach();
} else {
// Click handler for the "Ok" element in the cookiewarning information bar
$( '.mw-cookiewarning-dismiss' ).on( 'click', function ( ev ) {
$( '.mw-cookiewarning-container button' ).on( 'click', function ( ev ) {
// an anonymous user doesn't have preferences, so don't try to save this in
// the user preferences.
if ( !mw.user.isAnon() ) {

View file

@ -1,49 +1,38 @@
@z-index: 1999;
/* stylelint-disable selector-max-id */
#siteNotice {
z-index: @z-index;
}
/* stylelint-enable selector-max-id */
.mw-cookiewarning-container {
position: fixed;
background-color: rgba( 90, 90, 90, 0.85 );
box-sizing: border-box;
color: #fff;
font-weight: bold;
font-size: 13px;
padding: 7px 15px;
top: 0;
bottom: 0;
left: 0;
width: 100%;
z-index: @z-index;
z-index: 1999;
display: flex;
justify-content: center;
a,
.mw-cookiewarning-dismiss {
background-color: #3c3c3c;
height: 100%;
padding: 3px 10px;
border-radius: 2px;
border: 0;
cursor: pointer;
text-decoration: none !important; /* stylelint-disable-line declaration-no-important */
color: #fff;
margin-right: 0.5em;
white-space: nowrap;
font-weight: bold;
font-size: 13px;
// Narrow mobile screens
// 550px is a mostly arbitrary number, though it's used by a few other exts and skins
@media screen and ( max-width: 550px ) {
flex-wrap: wrap;
.mw-cookiewarning-text {
margin-bottom: 0.7em;
}
}
.mw-cookiewarning-text {
display: inline-block;
span {
margin-right: 0.5em;
}
display: flex;
align-items: center;
color: #fff;
font-weight: bold;
font-size: 92%;
}
form {
display: inline-block;
flex-shrink: 0;
.oo-ui-widget {
margin: 0 0 0 8px;
}
}
}

View file

@ -1,65 +0,0 @@
@import 'mediawiki.ui/variables.less';
@cookieWarningFontSize: 12px;
@contentPaddingTablet: @iconGutterWidth + @iconGutterWidth + @iconSize;
.mw-cookiewarning-container {
background-color: #fff;
padding: 16px;
font-size: @cookieWarningFontSize;
font-weight: 500;
font-style: normal;
font-stretch: normal;
display: flex;
align-items: center;
justify-content: space-between;
.mw-cookiewarning-text {
display: flex;
align-items: center;
.mw-cookiewarning-cimage {
width: 28px;
height: 100%;
font-size: 28px;
margin-right: 15px;
float: left;
text-align: center;
line-height: 100%;
}
}
form {
margin-left: 5px;
/* borrowed from mediawiki/core Login Button, following `ButtonWidget (progressive)` from OOjs UI */
.mw-cookiewarning-dismiss {
height: 32px;
background-color: #f8f9fa;
color: #36c;
}
.mw-cookiewarning-dismiss:hover {
background-color: #fff;
border-color: #859ecc;
box-shadow: none;
}
.mw-cookiewarning-dismiss:active {
background-color: #eff3fa;
color: #2a4b8d;
border-color: #2a4b8d;
}
.mw-cookiewarning-dismiss:focus {
border-color: #36c;
box-shadow: inset 0 0 0 1px #36c;
}
}
}
@media all and ( min-width: @width-breakpoint-tablet ) {
.mw-cookiewarning-container {
line-height: @contentPaddingTablet;
.mw-cookiewarning-cimage {
line-height: inherit;
}
}
}

View file

@ -1,12 +0,0 @@
@import 'mediawiki.ui/variables.less';
.mw-cookiewarning-container {
border-bottom: 1px solid #c8ccd1;
padding: 2.5em;
}
@media all and ( min-width: @width-breakpoint-tablet ) {
.mw-cookiewarning-container {
line-height: normal;
}
}

View file

@ -2,19 +2,14 @@
namespace CookieWarning\Tests;
use CommentStoreComment;
use CookieWarning\GeoLocation;
use CookieWarning\Hooks;
use DerivativeContext;
use FauxRequest;
use MediaWiki\MediaWikiServices;
use MediaWiki\Revision\SlotRecord;
use MediaWikiLangTestCase;
use RequestContext;
use SkinTemplate;
use Title;
use WikiPage;
use WikitextContent;
/**
* @covers Hooks
@ -24,49 +19,78 @@ class HooksTest extends MediaWikiLangTestCase {
/**
* @dataProvider providerOnSiteNoticeAfter
*
* @param bool $enabled
* @param false|string $morelinkConfig
* @param false|string $morelinkCookieWarningMsg
* @param false|string $morelinkCookiePolicyMsg
* @param false|string $expectedLink
*
* @throws \MWException
* @throws \ConfigException
*/
public function testOnSiteNoticeAfter( $enabled, $morelinkConfig,
$morelinkCookieWarningMsg, $morelinkCookiePolicyMsg, $expectedLink
) {
public function testOnSiteNoticeAfter(
bool $enabled,
$morelinkConfig,
$morelinkCookieWarningMsg,
$morelinkCookiePolicyMsg,
$expectedLink
) : void {
$this->setMwGlobals( [
'wgCookieWarningEnabled' => $enabled,
'wgCookieWarningMoreUrl' => $morelinkConfig,
'wgCookieWarningForCountryCodes' => false,
'wgUseMediaWikiUIEverywhere' => true,
] );
MediaWikiServices::getInstance()->getMessageCache()->enable();
if ( $morelinkCookieWarningMsg ) {
$title = Title::newFromText( 'cookiewarning-more-link', NS_MEDIAWIKI );
$wikiPage = WikiPage::factory( $title );
$pageUpdater = $wikiPage->newPageUpdater( \User::newFromName( 'UTSysop' ) );
$pageUpdater->setContent( SlotRecord::MAIN, new WikitextContent( $morelinkCookieWarningMsg ) );
$pageUpdater->saveRevision( CommentStoreComment::newUnsavedComment( 'CookieWarning test' ) );
$this->editPage(
'cookiewarning-more-link',
strval( $morelinkCookieWarningMsg ),
'',
NS_MEDIAWIKI,
$this->getTestSysop()->getUser()
);
}
if ( $morelinkCookiePolicyMsg ) {
$title = Title::newFromText( 'cookie-policy-link', NS_MEDIAWIKI );
$wikiPage = WikiPage::factory( $title );
$pageUpdater = $wikiPage->newPageUpdater( \User::newFromName( 'UTSysop' ) );
$pageUpdater->setContent( SlotRecord::MAIN, new WikitextContent( $morelinkCookiePolicyMsg ) );
$pageUpdater->saveRevision( CommentStoreComment::newUnsavedComment( 'CookieWarning test' ) );
}
$sk = new SkinTemplate();
$data = '';
Hooks::onSiteNoticeAfter( $data, $sk );
if ( $expectedLink === false ) {
$expected = '';
} else {
// @codingStandardsIgnoreStart Generic.Files.LineLength
$expected =
str_replace( '$1', $expectedLink,
'<div class="mw-cookiewarning-container banner-container"><div class="mw-cookiewarning-text"><span>Cookies help us deliver our services. By using our services, you agree to our use of cookies.</span>$1</div><form method="POST"><input name="disablecookiewarning" class="mw-cookiewarning-dismiss mw-ui-button" type="submit" value="OK"/></form></div>' );
// @codingStandardsIgnoreEnd
}
$this->assertEquals( $expected, $data );
$this->editPage(
'cookie-policy-link',
strval( $morelinkCookiePolicyMsg ),
'',
NS_MEDIAWIKI,
$this->getTestSysop()->getUser()
);
}
public function providerOnSiteNoticeAfter() {
$sk = new SkinTemplate();
// setup OOUI, that would be normally done in BeforePageDisplay hook
$sk->getOutput()->enableOOUI();
$data = '';
Hooks::onSkinAfterContent( $data, $sk );
if ( $enabled ) {
$this->assertNotEmpty( $data, 'Cookie warning should be present' );
} else {
$this->assertEmpty( $data, 'Cookie warning should not be present' );
return;
}
if ( $expectedLink === false ) {
$this->assertNotRegExp(
'/<a[^>]+href=[\'"]([^\'"]+)[\'"].+?>/',
$data,
'More information link should not be present'
);
} else {
$this->assertRegExp(
'/<a[^>]+href=[\'"]' . preg_quote( $expectedLink, '/' ) . '[\'"].+?>/',
$data,
'More information link should be present and match the expectation'
);
}
}
public function providerOnSiteNoticeAfter() : array {
return [
[
// $wgCookieWarningEnabled
@ -78,7 +102,7 @@ class HooksTest extends MediaWikiLangTestCase {
// MediaWiki:Cookie-policy-link
false,
// expected cookie warning link (when string), nothing if false
'',
false,
],
[
false,
@ -92,21 +116,21 @@ class HooksTest extends MediaWikiLangTestCase {
'http://google.de',
false,
false,
"\u{00A0}<a href=\"http://google.de\">More information</a>",
'http://google.de',
],
[
true,
'',
'http://google.de',
false,
"\u{00A0}<a href=\"http://google.de\">More information</a>",
'http://google.de',
],
[
true,
'',
false,
'http://google.de',
"\u{00A0}<a href=\"http://google.de\">More information</a>",
'http://google.de',
],
// the config should be the used, if set (no matter if the messages are used or not)
[
@ -114,21 +138,21 @@ class HooksTest extends MediaWikiLangTestCase {
'http://google.de',
false,
'http://google123.de',
"\u{00A0}<a href=\"http://google.de\">More information</a>",
'http://google.de',
],
[
true,
'http://google.de',
'http://google1234.de',
'http://google123.de',
"\u{00A0}<a href=\"http://google.de\">More information</a>",
'http://google.de',
],
[
true,
'',
'http://google.de',
'http://google123.de',
"\u{00A0}<a href=\"http://google.de\">More information</a>",
'http://google.de',
],
];
}
@ -155,7 +179,7 @@ class HooksTest extends MediaWikiLangTestCase {
$sk = new SkinTemplate();
$sk->setContext( $context );
$data = '';
Hooks::onSiteNoticeAfter( $data, $sk );
Hooks::onSkinAfterContent( $data, $sk );
$this->assertEquals(
$expected,