From 5e3808a14e83b2a656eeddb41bcd59f192c7baff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Tisza?= Date: Tue, 17 Jun 2014 22:23:18 +0000 Subject: [PATCH] Make the metadata panel opening affordance more obvious - rearrange DOM structure of above-fold part of the metadata panel: - rename .mw-mmv-controls to .mw-mmv-above-fold - the above-fold part is a single positioned div now, with height explitcitly set - less LESS gymnastics, above-fold height is a single variable - add paddings to the p elements instead of the containers - make all title elements align to baseline (except the logo which would look horrible) - discard some CSS which was superfluous - overspecified sizes/positions - some top/bottoms for staticly positioned elements - get rid of the .mw-mmv-drag-affordance div, since a full-width bar wouldn't really make sense on the bottom of the above-fold section - flip the chevron and place it to the bottom of the above-fold part; add colors etc. per spec - fix stripe button horizontal spacing Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/706 Change-Id: Ic37b4150288055c3fae8d22919ed7b1249db1f09 --- resources/mmv/mmv.globals.less | 9 +- resources/mmv/mmv.lightboxinterface.js | 2 +- resources/mmv/mmv.lightboxinterface.less | 34 +++---- resources/mmv/ui/img/drag-active.svg | 33 +++++-- resources/mmv/ui/img/drag.svg | 33 +++++-- resources/mmv/ui/mmv.ui.canvasButtons.less | 2 +- resources/mmv/ui/mmv.ui.metadataPanel.less | 19 ++-- .../mmv/ui/mmv.ui.metadataPanelScroller.js | 68 +++++++------- .../mmv/ui/mmv.ui.metadataPanelScroller.less | 92 +++++++++++++------ resources/mmv/ui/mmv.ui.progressBar.less | 3 +- resources/mmv/ui/mmv.ui.stripeButtons.less | 10 +- .../ui/mmv.ui.metadataPanelScroller.test.js | 30 +++--- 12 files changed, 190 insertions(+), 145 deletions(-) diff --git a/resources/mmv/mmv.globals.less b/resources/mmv/mmv.globals.less index 0d4dfb37d..f3574f376 100644 --- a/resources/mmv/mmv.globals.less +++ b/resources/mmv/mmv.globals.less @@ -1,6 +1,5 @@ -// Height of the drag affordance on the top of the metadata bar. -@metadatabar-drag-height: 18px; +// Height of the area of the metadata bar which is visible without scrolling. +@metadatabar-above-fold-height: 82px; -// Height of the top content area of the metadata bar (.mw-mmv-title-contain). Together with -// the drag affordance, these two constitute the above-the-fold part of the metadata bar. -@metadatabar-top-content-height: 64px; +// Height of the same area in fullscreen mode (which can be slightly less since it has less controls). +@metadatabar-above-fold-fullscreen-height: 64px; diff --git a/resources/mmv/mmv.lightboxinterface.js b/resources/mmv/mmv.lightboxinterface.js index 7b98572ff..ddcfd5e01 100644 --- a/resources/mmv/mmv.lightboxinterface.js +++ b/resources/mmv/mmv.lightboxinterface.js @@ -73,7 +73,7 @@ .addClass( 'mw-mmv-post-image' ); this.$controlBar = $( '
' ) - .addClass( 'mw-mmv-controls' ); + .addClass( 'mw-mmv-above-fold' ); this.$main.append( this.$preDiv, diff --git a/resources/mmv/mmv.lightboxinterface.less b/resources/mmv/mmv.lightboxinterface.less index 96591580c..d0cc57b46 100644 --- a/resources/mmv/mmv.lightboxinterface.less +++ b/resources/mmv/mmv.lightboxinterface.less @@ -1,7 +1,6 @@ @import "mmv.globals"; @import "mmv.mixins"; -@bottom-height: (@metadatabar-top-content-height + @metadatabar-drag-height); @metadata-background: rgb(251, 251, 251); .mw-mmv-wrapper { @@ -23,14 +22,10 @@ } } -.mw-mmv-image-wrapper, -.mw-mmv-controls { - top: 0px; - bottom: @bottom-height; -} - .mw-mmv-image-wrapper { position: fixed; + top: 0px; + bottom: @metadatabar-above-fold-height; left: 0px; right: 0px; } @@ -56,17 +51,19 @@ height: auto; color: #333333; background-color: @metadata-background; - min-height: (@bottom-height + 1); + min-height: (@metadatabar-above-fold-height + 1); z-index: 2; } -.mw-mmv-controls { +// above-the-fold part of the metadata panel +.mw-mmv-above-fold { width: 100%; - height: auto; + height: @metadatabar-above-fold-height; + position: relative; border-bottom: 1px solid #cccccc; } -/* Fullscreen styles */ +// Fullscreen styles .cursor-hidden { cursor: none; @@ -77,22 +74,17 @@ } .jq-fullscreened { - .mw-mmv-image-wrapper, - .mw-mmv-post-image, - .mw-mmv-controls { + .mw-mmv-image-wrapper, // make the image occupy the whole screen + .mw-mmv-post-image { // make sure the panel fits in the screen and does not cause scrollbars to appear bottom: 0px; } - .mw-mmv-post-image, - .mw-mmv-controls { - @fullscreen-padding: 10px; - padding: (@fullscreen-padding / 2) 0; - height: (@metadatabar-top-content-height + @fullscreen-padding); - min-height: 0; + .mw-mmv-above-fold { + height: @metadatabar-above-fold-fullscreen-height; } .mw-mmv-post-image { - position: fixed; + min-height: 0; .opacity(0); transition: opacity 0.25s; diff --git a/resources/mmv/ui/img/drag-active.svg b/resources/mmv/ui/img/drag-active.svg index 2264c5096..5843f0056 100644 --- a/resources/mmv/ui/img/drag-active.svg +++ b/resources/mmv/ui/img/drag-active.svg @@ -1,9 +1,24 @@ - - - - - - + + + +image/svg+xml + + \ No newline at end of file diff --git a/resources/mmv/ui/img/drag.svg b/resources/mmv/ui/img/drag.svg index a380d91df..c116cf4ad 100644 --- a/resources/mmv/ui/img/drag.svg +++ b/resources/mmv/ui/img/drag.svg @@ -1,9 +1,24 @@ - - - - - - + + + +image/svg+xml + + \ No newline at end of file diff --git a/resources/mmv/ui/mmv.ui.canvasButtons.less b/resources/mmv/ui/mmv.ui.canvasButtons.less index 640fbff9d..deab626b1 100644 --- a/resources/mmv/ui/mmv.ui.canvasButtons.less +++ b/resources/mmv/ui/mmv.ui.canvasButtons.less @@ -82,7 +82,7 @@ } .mw-mmv-viewfile { - bottom: (@buttons-offset + @metadatabar-drag-height + @metadatabar-top-content-height); + bottom: (@buttons-offset + @metadatabar-above-fold-height); /* @embed */ background-image: url(img/mw-open-control-ltr.svg); width: 23px; diff --git a/resources/mmv/ui/mmv.ui.metadataPanel.less b/resources/mmv/ui/mmv.ui.metadataPanel.less index f979396c9..3e4d84772 100644 --- a/resources/mmv/ui/mmv.ui.metadataPanel.less +++ b/resources/mmv/ui/mmv.ui.metadataPanel.less @@ -22,15 +22,13 @@ position: relative; } -.mw-mmv-license, -.mw-mmv-title-contain { - vertical-align: middle; -} - .mw-mmv-title-para { - margin-bottom: 1px; - margin-top: 0px; - padding: 0px; + margin: 0; + padding: 10px 0 0; +} +.mw-mmv-credit { + margin: 0; + padding: 5px 0; } .mw-mmv-title { @@ -166,7 +164,10 @@ } .mw-mmv-title-credit { - height: @metadatabar-top-content-height; + height: @metadatabar-above-fold-height; + .jq-fullscreened & { + height: @metadatabar-above-fold-fullscreen-height; + } } .mw-mmv-license { diff --git a/resources/mmv/ui/mmv.ui.metadataPanelScroller.js b/resources/mmv/ui/mmv.ui.metadataPanelScroller.js index bcc81fe26..c677f120b 100644 --- a/resources/mmv/ui/mmv.ui.metadataPanelScroller.js +++ b/resources/mmv/ui/mmv.ui.metadataPanelScroller.js @@ -32,12 +32,19 @@ /** @property {Object} localStorage the window.localStorage object */ this.localStorage = localStorage; + /** + * Whether this user has ever opened the metadata panel. + * Based on a localstorage flag; will be set to true if the client does not support localstorage. + * @type {boolean} + */ + this.hasOpenedMetadata = undefined; + /** * Whether we've already fired an animation for the metadata div in this lightbox session. * @property {boolean} * @private */ - this.hasAnimatedMetadata = undefined; + this.hasAnimatedMetadata = false; this.initialize(); } @@ -58,7 +65,7 @@ } ) ); // reset animation flag when the viewer is reopened - this.hasAnimatedMetadata = !this.localStorage || this.localStorage.getItem( 'mmv.hasOpenedMetadata' ); + this.hasAnimatedMetadata = false; }; MPSP.unattach = function() { @@ -67,7 +74,7 @@ }; MPSP.empty = function () { - this.$dragIcon.removeClass( 'pointing-down' ); + this.$dragIcon.toggleClass( 'panel-never-opened', !this.hasOpenedMetadata ); // need to remove this to avoid animating again when reopening lightbox on same page this.$container.removeClass( 'invite' ); @@ -76,32 +83,33 @@ MPSP.initialize = function () { var panel = this; - this.dragBarGravity = 's'; - - this.$dragBar = $( '
' ) + this.$dragIcon = $( '
' ) + .addClass( 'mw-mmv-drag-icon mw-mmv-drag-icon-pointing-down' ) + .toggleClass( 'panel-never-opened', !this.hasOpenedMetadata ) .prop( 'title', mw.message( 'multimediaviewer-panel-open-popup-text' ).text() ) - .tipsy( { - delayIn: mw.config.get( 'wgMultimediaViewer').tooltipDelay, - gravity: function () { - return panel.dragBarGravity; - } - } ) - .addClass( 'mw-mmv-drag-affordance' ) + .tipsy( { gravity: 's', delayIn: mw.config.get( 'wgMultimediaViewer').tooltipDelay } ) .appendTo( this.$controlBar ) .click( function () { panel.toggle(); } ); - this.$dragIcon = $( '
' ) - .addClass( 'mw-mmv-drag-icon' ) - .appendTo( this.$dragBar ); + this.$dragIconBottom = $( '
' ) + .addClass( 'mw-mmv-drag-icon mw-mmv-drag-icon-pointing-up' ) + .prop( 'title', mw.message( 'multimediaviewer-panel-close-popup-text' ).text() ) + .tipsy( { gravity: 's', delayIn: mw.config.get( 'wgMultimediaViewer').tooltipDelay } ) + .appendTo( this.$container ) + .click( function () { + panel.toggle(); + } ); + + this.hasOpenedMetadata = !this.localStorage || this.localStorage.getItem( 'mmv.hasOpenedMetadata' ); }; /** * Animates the metadata area when the viewer is first opened. */ MPSP.animateMetadataOnce = function () { - if ( !this.hasAnimatedMetadata ) { + if ( !this.hasOpenedMetadata && !this.hasAnimatedMetadata ) { this.hasAnimatedMetadata = true; this.$container.addClass( 'invite' ); } @@ -124,17 +132,6 @@ mw.mmv.actionLogger.log( wasOpen ? 'metadata-open' : 'metadata-close' ); - this.$dragBar - .prop( 'title', - mw.message( - 'multimediaviewer-panel-' + - ( !wasOpen ? 'open' : 'close' ) + - '-popup-text' - ).text() - ); - - this.dragBarGravity = wasOpen ? 'n' : 's'; - $.scrollTo( scrollTopTarget, this.toggleScrollDuration ); }; @@ -183,21 +180,20 @@ MPSP.scroll = function () { var scrolled = !!$.scrollTo().scrollTop(); - this.$dragIcon.toggleClass( 'pointing-down', scrolled ); + this.$dragIcon.toggleClass( 'panel-open', scrolled ); if ( - !this.savedHasOpenedMetadata && - scrolled && - this.localStorage - ) { + !this.hasOpenedMetadata + && scrolled + && this.localStorage + ) { + this.hasOpenedMetadata = true; + this.$dragIcon.removeClass( 'panel-never-opened' ); try { this.localStorage.setItem( 'mmv.hasOpenedMetadata', true ); } catch ( e ) { // localStorage is full or disabled } - - // We mark it as saved even when localStorage failed, because retrying will very likely fail as well - this.savedHasOpenedMetadata = true; } }; diff --git a/resources/mmv/ui/mmv.ui.metadataPanelScroller.less b/resources/mmv/ui/mmv.ui.metadataPanelScroller.less index c439d11e2..58825cdba 100644 --- a/resources/mmv/ui/mmv.ui.metadataPanelScroller.less +++ b/resources/mmv/ui/mmv.ui.metadataPanelScroller.less @@ -2,6 +2,11 @@ @import "../mmv.mixins"; @import "mediawiki.mixins.animation"; +@drag-icon-width: 64px; +@drag-icon-height: 18px; +@drag-icon-color: #e6e6e6; +@drag-icon-invite-color: #347bff; + .mw-mmv-post-image { .animation( mw-mmv-appear-animation 0.5s ease 0s 1 normal forwards ); &.invite { @@ -69,39 +74,66 @@ .mw-mmv-invite-animation; } -.mw-mmv-drag-affordance { - width: 100%; - height: @metadatabar-drag-height; +.mw-mmv-drag-icon { + width: @drag-icon-width; + height: @drag-icon-height; + + position: absolute; + left: 50%; + bottom: 0; + margin: 0 0 0 -(@drag-icon-width / 2); + + /* @embed */ + background-image: url(img/drag.svg); + background-repeat: no-repeat; + background-position: center bottom; + background-color: @drag-icon-color; + cursor: pointer; + z-index: 1; // make sure it is above the text - the icon is visually at the bottom but in the DOM at the top + .opacity(0.7); + transition: opacity 0.25s; + + &-pointing-down { // use single-class selector - chevron direction is important enough to make it IE6-compatible + background-position: center top; + .rotate(180deg); + + -webkit-border-bottom-left-radius: 3px; + -webkit-border-bottom-right-radius: 3px; + -moz-border-radius-bottomleft: 3px; + -moz-border-radius-bottomright: 3px; + border-bottom-left-radius: 3px; + border-bottom-right-radius: 3px; + + .mw-mmv-post-image:hover & { + .opacity(1); + } + } + &-pointing-up { + -webkit-border-top-left-radius: 3px; + -webkit-border-top-right-radius: 3px; + -moz-border-radius-topleft: 3px; + -moz-border-radius-topright: 3px; + border-top-left-radius: 3px; + border-top-right-radius: 3px; + + &:hover { + .opacity(1); + } + } + + &.panel-open { + display: none; + } + + &.panel-never-opened { + /* @embed */ + background-image: url(img/drag-active.svg); + background-color: @drag-icon-invite-color; + .opacity(1); + } .jq-fullscreened & { display: none; } } - -.mw-mmv-drag-icon { - width: 64px; - height: @metadatabar-drag-height; - /* @embed */ - background-image: url(img/drag.svg); - background-repeat: no-repeat; - background-position: center bottom; - margin: 0 auto; - .opacity(0.7); - transition: opacity 0.25s; - - &.pointing-down { - background-position: center top; - .rotate(180deg); - } - - .mw-mmv-post-image.invite & { - /* @embed */ - background-image: url(img/drag-active.svg); - .opacity(0.9); - } -} - -.mw-mmv-post-image:hover .mw-mmv-drag-icon { - .opacity(1); -} diff --git a/resources/mmv/ui/mmv.ui.progressBar.less b/resources/mmv/ui/mmv.ui.progressBar.less index 147c19d12..1473e3642 100644 --- a/resources/mmv/ui/mmv.ui.progressBar.less +++ b/resources/mmv/ui/mmv.ui.progressBar.less @@ -5,9 +5,10 @@ .mw-mmv-progress { width: 100%; height: @progress-height; + position: relative; + bottom: @progress-height; background-color: #cccccc; background-color: rgba( 221, 221, 221, 0.5 ); - margin-top: -@progress-height; } .mw-mmv-progress.empty { diff --git a/resources/mmv/ui/mmv.ui.stripeButtons.less b/resources/mmv/ui/mmv.ui.stripeButtons.less index ba360b9d4..c8a9795fa 100644 --- a/resources/mmv/ui/mmv.ui.stripeButtons.less +++ b/resources/mmv/ui/mmv.ui.stripeButtons.less @@ -14,7 +14,10 @@ float: right; height: @button-height; - margin-top: @metadatabar-top-content-height - ( @button-height + 2 * @button-vertical-padding ); + margin-top: @metadatabar-above-fold-height - ( @button-height + 2 * @button-vertical-padding ); + .jq-fullscreened & { + margin-top: @metadatabar-above-fold-fullscreen-height - ( @button-height + 2 * @button-vertical-padding ); + } border-left: 1px solid @border-text-color; padding: @button-vertical-padding 20px; @@ -22,7 +25,7 @@ font-size: 1.25em; color: @button-text-color; cursor: pointer; - opacity: 0.8; + .opacity(0.8); transition: opacity 0.25s; // when the button is a link, we need more selector specificity @@ -36,7 +39,7 @@ &.open, &:hover { - opacity: 1; + .opacity(1); } &:before { @@ -49,7 +52,6 @@ top: 0.1em; background-size: 100% 100%; - margin-right: 0.25em; content: ' '; vertical-align: baseline; } diff --git a/tests/qunit/mmv/ui/mmv.ui.metadataPanelScroller.test.js b/tests/qunit/mmv/ui/mmv.ui.metadataPanelScroller.test.js index 663cd7ef1..8077da5f3 100644 --- a/tests/qunit/mmv/ui/mmv.ui.metadataPanelScroller.test.js +++ b/tests/qunit/mmv/ui/mmv.ui.metadataPanelScroller.test.js @@ -22,12 +22,11 @@ } } ) ); - QUnit.test( 'empty()', 2, function ( assert ) { + QUnit.test( 'empty()', 1, function ( assert ) { var $qf = $( '#qunit-fixture' ), scroller = new mw.mmv.ui.MetadataPanelScroller( $qf, $( '
' ).appendTo( $qf ) ); scroller.empty(); - assert.ok( !scroller.$dragIcon.hasClass( 'pointing-down' ), 'We successfully reset the chevron' ); assert.ok( !scroller.$container.hasClass( 'invite' ), 'We successfully reset the invite' ); } ); @@ -76,19 +75,23 @@ scroller.scroll(); - assert.ok( !scroller.savedHasOpenedMetadata, 'No localStorage, we don\'t try to save the opened flag' ); + assert.strictEqual( scroller.hasOpenedMetadata, true, 'We store hasOpenedMetadata flag for the session' ); } ); - QUnit.test( 'localStorage is full', 1, function( assert ) { + QUnit.test( 'localStorage is full', 2, function( assert ) { var $qf = $( '#qunit-fixture' ), - scroller = new mw.mmv.ui.MetadataPanelScroller( $qf, $( '
' ).appendTo( $qf ), - { getItem : $.noop, setItem : function() { throw 'I am full'; } } ); + localStorage = { getItem : $.noop, setItem : this.sandbox.stub().throwsException( 'I am full' ) }, + scroller = new mw.mmv.ui.MetadataPanelScroller( $qf, $( '
' ).appendTo( $qf ), localStorage ); this.sandbox.stub( $, 'scrollTo', function() { return { scrollTop : function() { return 10; } }; } ); scroller.scroll(); - assert.ok( scroller.savedHasOpenedMetadata, 'Full localStorage, we don\'t try to save the opened flag more than once' ); + assert.strictEqual( scroller.hasOpenedMetadata, true, 'We store hasOpenedMetadata flag for the session' ); + + scroller.scroll(); + + assert.ok( localStorage.setItem.calledOnce, 'localStorage only written once' ); } ); /** @@ -135,7 +138,7 @@ } ); } - QUnit.test( 'Metadata scrolling', 12, function ( assert ) { + QUnit.test( 'Metadata scrolling', 7, function ( assert ) { var $qf = $( '#qunit-fixture' ), $container = $( '
' ).css( 'height', 100 ).appendTo( $qf ), $controlBar = $( '
' ).css( 'height', 50 ).appendTo( $container ), @@ -154,8 +157,6 @@ scroller.attach(); assert.strictEqual( $.scrollTo().scrollTop(), 0, 'scrollTo scrollTop should be set to 0' ); - assert.ok( !scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing up' ); assert.ok( !fakeLocalStorage.setItem.called, 'The metadata hasn\'t been open yet, no entry in localStorage' ); @@ -163,8 +164,6 @@ scroller.keydown( keydown ); this.clock.tick( scroller.toggleScrollDuration ); - assert.ok( scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing down after pressing up arrow' ); assert.ok( fakeLocalStorage.setItem.calledWithExactly( 'mmv.hasOpenedMetadata', true ), 'localStorage knows that the metadata has been open' ); keydown.which = 40; // Down arrow @@ -173,22 +172,15 @@ assert.strictEqual( $.scrollTo().scrollTop(), 0, 'scrollTo scrollTop should be set to 0 after pressing down arrow' ); - assert.ok( !scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing up after pressing down arrow' ); scroller.$dragIcon.click(); this.clock.tick( scroller.toggleScrollDuration ); - assert.ok( scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing down after clicking the chevron once' ); - scroller.$dragIcon.click(); this.clock.tick( scroller.toggleScrollDuration ); assert.strictEqual( $.scrollTo().scrollTop(), 0, 'scrollTo scrollTop should be set to 0 after clicking the chevron twice' ); - assert.ok( !scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing up after clicking the chevron twice' ); // Unattach lightbox from document scroller.unattach();