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
This commit is contained in:
jdlrobson 2017-12-12 17:25:10 -08:00
parent 1648c7752e
commit 2e453edd23
5 changed files with 94 additions and 82 deletions

View File

@ -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;
}

View File

@ -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,

View File

Before

Width:  |  Height:  |  Size: 879 B

After

Width:  |  Height:  |  Size: 879 B

View File

@ -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;
}
}

View File

@ -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",