Gilles has uploaded a new change for review. https://gerrit.wikimedia.org/r/133893
Change subject: Highlight chevron when the wrong direction is pressed ...................................................................... Highlight chevron when the wrong direction is pressed Change-Id: I0d43c58a16fa805611f9fdef329b5ab6a32ed651 Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/554 --- M resources/mmv/ui/mmv.ui.metadataPanelScroller.js M resources/mmv/ui/mmv.ui.metadataPanelScroller.less M tests/qunit/mmv/mmv.lightboxinterface.test.js M tests/qunit/mmv/ui/mmv.ui.metadataPanelScroller.test.js 4 files changed, 203 insertions(+), 181 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MultimediaViewer refs/changes/93/133893/1 diff --git a/resources/mmv/ui/mmv.ui.metadataPanelScroller.js b/resources/mmv/ui/mmv.ui.metadataPanelScroller.js index ceaf853..33b10fc 100644 --- a/resources/mmv/ui/mmv.ui.metadataPanelScroller.js +++ b/resources/mmv/ui/mmv.ui.metadataPanelScroller.js @@ -39,10 +39,19 @@ */ this.hasAnimatedMetadata = undefined; + /** + * Timer used to highlight the chevron when the wrong key is pressed + * @property {number} + * @private + */ + this.highlightTimeout = undefined; + this.initialize(); } oo.inheritClass( MetadataPanelScroller, mw.mmv.ui.Element ); MPSP = MetadataPanelScroller.prototype; + + MPSP.highlightDuration = 500; MPSP.attach = function() { var panel = this; @@ -100,7 +109,8 @@ * Toggles the metadata div being totally visible. */ MPSP.toggle = function ( forceDirection ) { - var scrollTopWhenOpen = this.$container.outerHeight() - this.$controlBar.outerHeight(), + var self = this, + scrollTopWhenOpen = this.$container.outerHeight() - this.$controlBar.outerHeight(), scrollTopWhenClosed = 0, scrollTop = $.scrollTo().scrollTop(), panelIsOpen = scrollTop > scrollTopWhenClosed, @@ -110,7 +120,13 @@ scrollTopTarget = forceDirection === 'down' ? scrollTopWhenClosed : scrollTopWhenOpen; if ( scrollTop === scrollTopTarget ) { // The user pressed down when the panel was closed already (or up when fully open). - // Not a real toggle; do not log. + // Not a real toggle; highlight the chevron to attract attention. + this.$container.addClass( 'mw-mmv-highlight-chevron' ); + + if ( this.highlightTimeout ) { + clearTimeout( this.highlightTimeout ); + } + this.highlightTimeout = setTimeout( function() { self.$container.removeClass( 'mw-mmv-highlight-chevron' ); }, self.highlightDuration ); return; } } diff --git a/resources/mmv/ui/mmv.ui.metadataPanelScroller.less b/resources/mmv/ui/mmv.ui.metadataPanelScroller.less index 1facb3a..0c290d0 100644 --- a/resources/mmv/ui/mmv.ui.metadataPanelScroller.less +++ b/resources/mmv/ui/mmv.ui.metadataPanelScroller.less @@ -86,7 +86,9 @@ background-position: center top; .rotate(180deg); } - .mw-mmv-post-image.invite & { + + .mw-mmv-post-image.invite &, .mw-mmv-post-image.mw-mmv-highlight-chevron & { + /* @embed */ background-image: url(img/drag-active.svg); opacity: 0.9; } diff --git a/tests/qunit/mmv/mmv.lightboxinterface.test.js b/tests/qunit/mmv/mmv.lightboxinterface.test.js index 0987c39..00e477c 100644 --- a/tests/qunit/mmv/mmv.lightboxinterface.test.js +++ b/tests/qunit/mmv/mmv.lightboxinterface.test.js @@ -240,183 +240,6 @@ restoreScrollTo(); } ); - /** - * We need to set up a proxy on the jQuery scrollTop function and the jQuery.scrollTo plugin, - * that will let us pretend that the document really scrolled and that will return values - * as if the scroll happened. - * @param {sinon.sandbox} sandbox - * @param {mw.mmv.LightboxInterface} ui - */ - function stubScrollFunctions( sandbox, ui ) { - var memorizedScrollToScroll = 0, - originalJQueryScrollTop = $.fn.scrollTop, - originalJQueryScrollTo = $.scrollTo; - - sandbox.stub( $.fn, 'scrollTop', function ( scrollTop ) { - // On some browsers $.scrollTo() != $document - if ( $.scrollTo().is( this ) ) { - if ( scrollTop !== undefined ) { - memorizedScrollToScroll = scrollTop; - return this; - } else { - return memorizedScrollToScroll; - } - } - - return originalJQueryScrollTop.call( this, scrollTop ); - } ); - - sandbox.stub( $, 'scrollTo', function ( scrollTo ) { - var $element; - - if ( scrollTo !== undefined ) { - memorizedScrollToScroll = scrollTo; - } - - $element = originalJQueryScrollTo.call( this, scrollTo, 0 ); - - if ( scrollTo !== undefined ) { - // Trigger event manually - ui.panel.scroller.scroll(); - } - - return $element; - } ); - } - - QUnit.test( 'Metadata scrolling', 14, function ( assert ) { - var ui = new mw.mmv.LightboxInterface(), - keydown = $.Event( 'keydown' ), - $document = $( document ); - - stubScrollFunctions( this.sandbox, ui ); - - // First phase of the test: up and down arrows - - ui.panel.scroller.hasAnimatedMetadata = false; - localStorage.removeItem( 'mmv.hasOpenedMetadata' ); - - // Attach lightbox to testing fixture to avoid interference with other tests. - ui.attach( '#qunit-fixture' ); - - assert.strictEqual( $.scrollTo().scrollTop(), 0, 'scrollTo scrollTop should be set to 0' ); - assert.ok( !ui.panel.scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing up' ); - - assert.ok( !localStorage.getItem( 'mmv.hasOpenedMetadata' ), - 'The metadata hasn\'t been open yet, no entry in localStorage' ); - - keydown.which = 40; // Down arrow - $document.trigger( keydown ); - - keydown.which = 38; // Up arrow - $document.trigger( keydown ); - - assert.strictEqual( Math.round( $.scrollTo().scrollTop() ), - ui.panel.$imageMetadata.outerHeight(), - 'scrollTo scrollTop should be set to the metadata height after pressing up arrow' ); - assert.ok( ui.panel.scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing down after pressing up arrow' ); - assert.ok( localStorage.getItem( 'mmv.hasOpenedMetadata' ), - 'localStorage knows that the metadata has been open' ); - - keydown.which = 40; // Down arrow - $document.trigger( keydown ); - - assert.strictEqual( $.scrollTo().scrollTop(), 0, - 'scrollTo scrollTop should be set to 0 after pressing down arrow' ); - assert.ok( !ui.panel.scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing up after pressing down arrow' ); - - ui.panel.scroller.$dragIcon.click(); - - assert.strictEqual( Math.round( $.scrollTo().scrollTop() ), - ui.panel.$imageMetadata.outerHeight(), - 'scrollTo scrollTop should be set to the metadata height after clicking the chevron once' ); - assert.ok( ui.panel.scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing down after clicking the chevron once' ); - - ui.panel.scroller.$dragIcon.click(); - - assert.strictEqual( $.scrollTo().scrollTop(), 0, - 'scrollTo scrollTop should be set to 0 after clicking the chevron twice' ); - assert.ok( !ui.panel.scroller.$dragIcon.hasClass( 'pointing-down' ), - 'Chevron pointing up after clicking the chevron twice' ); - - // Unattach lightbox from document - ui.unattach(); - - - // Second phase of the test: scroll memory - - // Attach lightbox to testing fixture to avoid interference with other tests. - ui.attach( '#qunit-fixture' ); - - // To make sure that the details are out of view, the lightbox is supposed to scroll to the top when open - assert.strictEqual( $.scrollTo().scrollTop(), 0, 'Page scrollTop should be set to 0' ); - - // Scroll down to check that the scrollTop memory doesn't affect prev/next (bug 59861) - $.scrollTo( 20, 0 ); - - // This extra attach() call simulates the effect of prev/next seen in bug 59861 - ui.attach( '#qunit-fixture' ); - - // The lightbox was already open at this point, the scrollTop should be left untouched - assert.strictEqual( $.scrollTo().scrollTop(), 20, 'Page scrollTop should be set to 20' ); - - // Unattach lightbox from document - ui.unattach(); - } ); - - QUnit.test( 'Metadata scroll logging', 6, function ( assert ) { - var ui = new mw.mmv.LightboxInterface(), - keydown = $.Event( 'keydown' ), - $document = $( document ); - - stubScrollFunctions( this.sandbox, ui ); - this.sandbox.stub( mw.mmv.logger, 'log' ); - - // Attach lightbox to testing fixture to avoid interference with other tests. - ui.attach( '#qunit-fixture' ); - - keydown.which = 40; // Down arrow - $document.trigger( keydown ); - - assert.ok( !mw.mmv.logger.log.called, 'Closing keypress not logged when the panel is closed already' ); - mw.mmv.logger.log.reset(); - - keydown.which = 38; // Up arrow - $document.trigger( keydown ); - - assert.ok( mw.mmv.logger.log.calledWithExactly( 'metadata-open' ), 'Opening keypress logged' ); - mw.mmv.logger.log.reset(); - - keydown.which = 38; // Up arrow - $document.trigger( keydown ); - - assert.ok( !mw.mmv.logger.log.called, 'Opening keypress not logged when the panel is opened already' ); - mw.mmv.logger.log.reset(); - - keydown.which = 40; // Down arrow - $document.trigger( keydown ); - - assert.ok( mw.mmv.logger.log.calledWithExactly( 'metadata-close' ), 'Closing keypress logged' ); - mw.mmv.logger.log.reset(); - - ui.panel.scroller.$dragIcon.click(); - - assert.ok( mw.mmv.logger.log.calledWithExactly( 'metadata-open' ), 'Opening click logged' ); - mw.mmv.logger.log.reset(); - - ui.panel.scroller.$dragIcon.click(); - - assert.ok( mw.mmv.logger.log.calledWithExactly( 'metadata-close' ), 'Closing click logged' ); - mw.mmv.logger.log.reset(); - - // Unattach lightbox from document - ui.unattach(); - } ); - QUnit.test( 'Keyboard prev/next', 2, function ( assert ) { var viewer = new mw.mmv.MultimediaViewer(), lightbox = new mw.mmv.LightboxInterface(); diff --git a/tests/qunit/mmv/ui/mmv.ui.metadataPanelScroller.test.js b/tests/qunit/mmv/ui/mmv.ui.metadataPanelScroller.test.js index 45a380f..d0be0ce 100644 --- a/tests/qunit/mmv/ui/mmv.ui.metadataPanelScroller.test.js +++ b/tests/qunit/mmv/ui/mmv.ui.metadataPanelScroller.test.js @@ -16,7 +16,11 @@ */ ( function( mw, $ ) { - QUnit.module( 'mmv.ui.metadataPanelScroller', QUnit.newMwEnvironment() ); + QUnit.module( 'mmv.ui.metadataPanelScroller', QUnit.newMwEnvironment( { + setup: function () { + this.clock = this.sandbox.useFakeTimers(); + } + } ) ); QUnit.test( 'empty()', 2, function ( assert ) { var $qf = $( '#qunit-fixture' ), @@ -84,4 +88,181 @@ assert.ok( scroller.savedHasOpenedMetadata, 'Full localStorage, we don\'t try to save the opened flag more than once' ); } ); + + /** + * We need to set up a proxy on the jQuery scrollTop function and the jQuery.scrollTo plugin, + * that will let us pretend that the document really scrolled and that will return values + * as if the scroll happened. + * @param {sinon.sandbox} sandbox + * @param {mw.mmv.ui.MetadataPanelScroller} scroller + */ + function stubScrollFunctions( sandbox, scroller ) { + var memorizedScrollToScroll = 0, + originalJQueryScrollTop = $.fn.scrollTop, + originalJQueryScrollTo = $.scrollTo; + + sandbox.stub( $.fn, 'scrollTop', function ( scrollTop ) { + // On some browsers $.scrollTo() != $document + if ( $.scrollTo().is( this ) ) { + if ( scrollTop !== undefined ) { + memorizedScrollToScroll = scrollTop; + return this; + } else { + return memorizedScrollToScroll; + } + } + + return originalJQueryScrollTop.call( this, scrollTop ); + } ); + + sandbox.stub( $, 'scrollTo', function ( scrollTo ) { + var $element; + + if ( scrollTo !== undefined ) { + memorizedScrollToScroll = scrollTo; + } + + $element = originalJQueryScrollTo.call( this, scrollTo, 0 ); + + if ( scrollTo !== undefined ) { + // Trigger event manually + scroller.scroll(); + } + + return $element; + } ); + } + + QUnit.test( 'Metadata scrolling', 12, function ( assert ) { + var $qf = $( '#qunit-fixture' ), + $container = $( '<div>' ).css( 'height', 100 ).appendTo( $qf ), + $controlBar = $( '<div>' ).css( 'height', 50 ).appendTo( $container ), + fakeLocalStorage = { getItem : $.noop, setItem : $.noop }, + scroller = new mw.mmv.ui.MetadataPanelScroller( $container, $controlBar, fakeLocalStorage), + keydown = $.Event( 'keydown' ); + + stubScrollFunctions( this.sandbox, scroller ); + + this.sandbox.stub( fakeLocalStorage, 'setItem' ); + + // First phase of the test: up and down arrows + + scroller.hasAnimatedMetadata = false; + + 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' ); + + keydown.which = 40; // Down arrow + scroller.keydown( keydown ); + + keydown.which = 38; // Up arrow + scroller.keydown( keydown ); + + 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 + scroller.keydown( keydown ); + + 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(); + + assert.ok( scroller.$dragIcon.hasClass( 'pointing-down' ), + 'Chevron pointing down after clicking the chevron once' ); + + scroller.$dragIcon.click(); + + 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(); + + + // Second phase of the test: scroll memory + + scroller.attach(); + + // To make sure that the details are out of view, the lightbox is supposed to scroll to the top when open + assert.strictEqual( $.scrollTo().scrollTop(), 0, 'Page scrollTop should be set to 0' ); + + // Scroll down to check that the scrollTop memory doesn't affect prev/next (bug 59861) + $.scrollTo( 20, 0 ); + + // This extra attach() call simulates the effect of prev/next seen in bug 59861 + scroller.attach(); + + // The lightbox was already open at this point, the scrollTop should be left untouched + assert.strictEqual( $.scrollTo().scrollTop(), 20, 'Page scrollTop should be set to 20' ); + + scroller.unattach(); + } ); + + QUnit.test( 'Metadata scroll logging', 12, function ( assert ) { + var $qf = $( '#qunit-fixture' ), + $container = $( '<div>' ).css( 'height', 100 ).appendTo( $qf ), + $controlBar = $( '<div>' ).css( 'height', 50 ).appendTo( $container ), + scroller = new mw.mmv.ui.MetadataPanelScroller( $container, $controlBar ), + keydown = $.Event( 'keydown' ); + + stubScrollFunctions( this.sandbox, scroller ); + + this.sandbox.stub( mw.mmv.logger, 'log' ); + + assert.ok( !$container.hasClass( 'mw-mmv-highlight-chevron' ), 'Chevron is not highlighted' ); + + keydown.which = 40; // Down arrow + scroller.keydown( keydown ); + + assert.ok( !mw.mmv.logger.log.called, 'Closing keypress not logged when the panel is closed already' ); + assert.ok( $container.hasClass( 'mw-mmv-highlight-chevron' ), 'Chevron is highlighted' ); + this.clock.tick( scroller.highlightDuration ); + assert.ok( !$container.hasClass( 'mw-mmv-highlight-chevron' ), 'Chevron is not highlighted' ); + mw.mmv.logger.log.reset(); + + keydown.which = 38; // Up arrow + scroller.keydown( keydown ); + + assert.ok( mw.mmv.logger.log.calledWithExactly( 'metadata-open' ), 'Opening keypress logged' ); + mw.mmv.logger.log.reset(); + + assert.ok( !$container.hasClass( 'mw-mmv-highlight-chevron' ), 'Chevron is not highlighted' ); + + keydown.which = 38; // Up arrow + scroller.keydown( keydown ); + + assert.ok( !mw.mmv.logger.log.called, 'Opening keypress not logged when the panel is opened already' ); + assert.ok( $container.hasClass( 'mw-mmv-highlight-chevron' ), 'Chevron is highlighted' ); + this.clock.tick( scroller.highlightDuration ); + assert.ok( !$container.hasClass( 'mw-mmv-highlight-chevron' ), 'Chevron is not highlighted' ); + mw.mmv.logger.log.reset(); + + keydown.which = 40; // Down arrow + scroller.keydown( keydown ); + + assert.ok( mw.mmv.logger.log.calledWithExactly( 'metadata-close' ), 'Closing keypress logged' ); + mw.mmv.logger.log.reset(); + + scroller.$dragIcon.click(); + + assert.ok( mw.mmv.logger.log.calledWithExactly( 'metadata-open' ), 'Opening click logged' ); + mw.mmv.logger.log.reset(); + + scroller.$dragIcon.click(); + + assert.ok( mw.mmv.logger.log.calledWithExactly( 'metadata-close' ), 'Closing click logged' ); + mw.mmv.logger.log.reset(); + } ); }( mediaWiki, jQuery ) ); -- To view, visit https://gerrit.wikimedia.org/r/133893 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0d43c58a16fa805611f9fdef329b5ab6a32ed651 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/MultimediaViewer Gerrit-Branch: master Gerrit-Owner: Gilles <gdu...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
