From 2e453edd2357263a92c9a4849598cc0251e11d23 Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Tue, 12 Dec 2017 17:25:10 -0800 Subject: [PATCH] Only load notification icon (bell) CSS for logged in users * The notification-count style is only needed if you are logged in. Given a small percentage of our users are logged in, we load a lot of render blocking css unnecessarily. * The bell icon is not needed for anonymous users so pull that out from skins.minerva.icons.images which is loaded for all users into a module only used by logged in users (skins.minerva.icons.loggedin) * Simplify the user-button rule - it is overly specific - probably for historic reasons. Additional changes: * Simplify isAuthenticated helper Change-Id: Ia72e7e45d276e8aac1ff5471bf6158705c7b5f99 --- includes/skins/SkinMinerva.php | 6 +- resources/skins.minerva.base.styles/ui.less | 80 ------------------- .../bell.svg | 0 .../skins.minerva.loggedin.styles/styles.less | 73 +++++++++++++++++ skin.json | 17 +++- 5 files changed, 94 insertions(+), 82 deletions(-) rename resources/{skins.minerva.icons.images => skins.minerva.icons.loggedin}/bell.svg (100%) create mode 100644 resources/skins.minerva.loggedin.styles/styles.less diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php index 864f51c..3cc7e58 100644 --- a/includes/skins/SkinMinerva.php +++ b/includes/skins/SkinMinerva.php @@ -288,7 +288,7 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin { * @return bool */ protected function isAuthenticatedUser() { - return !$this->getUser()->isAnon(); + return $this->getUser()->isLoggedIn(); } /** @@ -1452,6 +1452,10 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin { $styles[] = 'skins.minerva.userpage.styles'; $styles[] = 'skins.minerva.userpage.icons'; } + if ( $this->isAuthenticatedUser() ) { + $styles[] = 'skins.minerva.loggedin.styles'; + $styles[] = 'skins.minerva.icons.loggedin'; + } return $styles; } diff --git a/resources/skins.minerva.base.styles/ui.less b/resources/skins.minerva.base.styles/ui.less index 14c84c2..c06d0c8 100644 --- a/resources/skins.minerva.base.styles/ui.less +++ b/resources/skins.minerva.base.styles/ui.less @@ -292,86 +292,6 @@ input.search { text-overflow: ellipsis; } -// FIXME: Create generic class to represent both of these headers -.overlay, -.header { - - // need to specify id or else other rules are more important - // FIXME: simplify when .icon class from Overlay used instead - #secondary-button.user-button, - .user-button { - // Make sure count is positioned correctly in relation to bell icon - position: relative; - - // can't use display:none class as icons must have a label to retain height - .label { - visibility: hidden; - } - - &.loading span { - display: none; - } - } -} - -.notification-count { - @circleSize: 24px; - @borderSize: 2px; - margin: auto; - height: @circleSize; - background: @notificationBackgroundRead; - color: @notificationColorRead; - cursor: pointer; - - .circle { - border-radius: 50%; - border: @borderSize solid @notificationColorRead; - margin: auto; - height: @circleSize - @borderSize; - width: @circleSize - @borderSize; - - /* stylelint-disable declaration-block-no-duplicate-properties */ - // Center the text number inside the circle - display: block; // Fallback for old iOS - text-align: center; // Fallback for old iOS - display: -webkit-box; - display: flex; - -webkit-box-align: center; - align-items: center; - -webkit-box-pack: center; - justify-content: center; - - span { - font-weight: bold; - font-size: 13px; - line-height: 1; - letter-spacing: -0.5px; - } - } - - &.notification-unseen { - color: @notificationColorUnread; - - .circle { - background: @notificationBackgroundUnread; - border-color: @notificationBackgroundUnread; - } - } - - // FIXME: There must be a better way of doing this - &.max { - right: 0.2em; - width: 2em; - height: 2em; - line-height: 2em; - font-size: 0.7em; - } - - &:hover { - text-decoration: none; - } -} - // This is here rather than in mainmenu.less because we want to load these rules for non-js users too // Transparent shield hidden by default .transparent-shield, diff --git a/resources/skins.minerva.icons.images/bell.svg b/resources/skins.minerva.icons.loggedin/bell.svg similarity index 100% rename from resources/skins.minerva.icons.images/bell.svg rename to resources/skins.minerva.icons.loggedin/bell.svg diff --git a/resources/skins.minerva.loggedin.styles/styles.less b/resources/skins.minerva.loggedin.styles/styles.less new file mode 100644 index 0000000..b459d1a --- /dev/null +++ b/resources/skins.minerva.loggedin.styles/styles.less @@ -0,0 +1,73 @@ +@import 'minerva.variables'; + +.user-button { + // Make sure count is positioned correctly in relation to bell icon + position: relative; + + // can't use display:none class as icons must have a label to retain height + .label { + visibility: hidden; + } + + &.loading span { + display: none; + } +} + +.notification-count { + @circleSize: 24px; + @borderSize: 2px; + margin: auto; + height: @circleSize; + background: @notificationBackgroundRead; + color: @notificationColorRead; + cursor: pointer; + + .circle { + border-radius: 50%; + border: @borderSize solid @notificationColorRead; + margin: auto; + height: @circleSize - @borderSize; + width: @circleSize - @borderSize; + + /* stylelint-disable declaration-block-no-duplicate-properties */ + // Center the text number inside the circle + display: block; // Fallback for old iOS + text-align: center; // Fallback for old iOS + display: -webkit-box; + display: flex; + -webkit-box-align: center; + align-items: center; + -webkit-box-pack: center; + justify-content: center; + + span { + font-weight: bold; + font-size: 13px; + line-height: 1; + letter-spacing: -0.5px; + } + } + + &.notification-unseen { + color: @notificationColorUnread; + + .circle { + background: @notificationBackgroundUnread; + border-color: @notificationBackgroundUnread; + } + } + + // FIXME: There must be a better way of doing this + &.max { + right: 0.2em; + width: 2em; + height: 2em; + line-height: 2em; + font-size: 0.7em; + } + + &:hover { + text-decoration: none; + } +} diff --git a/skin.json b/skin.json index acddca8..aef04c5 100644 --- a/skin.json +++ b/skin.json @@ -133,11 +133,17 @@ "resources/skins.minerva.tablet.styles/styles.less" ] }, + "skins.minerva.icons.loggedin": { + "class": "ResourceLoaderImageModule", + "selector": ".mw-ui-icon-minerva-{name}:before", + "images": { + "notifications": "resources/skins.minerva.icons.loggedin/bell.svg" + } + }, "skins.minerva.icons.images": { "class": "ResourceLoaderImageModule", "selector": ".mw-ui-icon-minerva-{name}:before, .mw-ui-icon-{name}:before", "images": { - "notifications": "resources/skins.minerva.icons.images/bell.svg", "mainmenu": "resources/skins.minerva.icons.images/hamburger.svg", "edit": "resources/skins.minerva.icons.images/editLocked.svg", "edit-enabled": "resources/skins.minerva.icons.images/edit.svg", @@ -245,6 +251,15 @@ "resources/skins.minerva.mainMenu/MainMenu.js" ] }, + "skins.minerva.loggedin.styles": { + "targets": [ + "mobile", + "desktop" + ], + "styles": [ + "resources/skins.minerva.loggedin.styles/styles.less" + ] + }, "skins.minerva.scripts": { "targets": [ "mobile",