From b0d885e4dca627bc6bcbe37f1953200b21f48ac1 Mon Sep 17 00:00:00 2001 From: Gilles Dubuc Date: Wed, 16 Jul 2014 11:24:07 -0400 Subject: [PATCH] Remove all survey-related code Change-Id: I67285260c13a1e8d3c37365bb80a7156c0fecd4e Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/794 --- MultimediaViewer.php | 7 - MultimediaViewerHooks.php | 3 +- i18n/en.json | 1 - i18n/qqq.json | 1 - resources/mmv/ui/mmv.ui.stripeButtons.js | 221 +----------------- resources/mmv/ui/mmv.ui.stripeButtons.less | 5 - .../qunit/mmv/ui/mmv.ui.stripeButtons.test.js | 146 +----------- 7 files changed, 4 insertions(+), 380 deletions(-) diff --git a/MultimediaViewer.php b/MultimediaViewer.php index dd81fd8cb..9c1c0afed 100644 --- a/MultimediaViewer.php +++ b/MultimediaViewer.php @@ -69,11 +69,6 @@ if ( !isset( $wgMediaViewerUseThumbnailGuessing ) ) { $wgMediaViewerUseThumbnailGuessing = true; } -if ( !isset( $wgMediaViewerShowSurvey ) ) { - /** @var bool: If set, MediaViewer might direct the user to a survey. **/ - $wgMediaViewerShowSurvey = false; -} - if ( !isset( $wgEnableMediaViewerForLoggedInUsersOnly ) ) { /** * @var bool: If set, and $wgMediaViewerIsInBeta is unset, Media Viewer will be turned on for @@ -388,8 +383,6 @@ $wgResourceModules += array( ), 'messages' => array( - 'multimediaviewer-feedback-button-text', - 'multimediaviewer-feedback-popup-text', 'multimediaviewer-description-page-button-text', 'multimediaviewer-description-page-popup-text', 'multimediaviewer-description-page-popup-text-local', diff --git a/MultimediaViewerHooks.php b/MultimediaViewerHooks.php index f442c16eb..27d7b49df 100644 --- a/MultimediaViewerHooks.php +++ b/MultimediaViewerHooks.php @@ -145,14 +145,13 @@ class MultimediaViewerHooks { */ public static function resourceLoaderGetConfigVars( &$vars ) { global $wgAPIPropModules, $wgMediaViewerActionLoggingSamplingFactorMap, $wgNetworkPerformanceSamplingFactor, $wgMediaViewerDurationLoggingSamplingFactor, - $wgMediaViewerIsInBeta, $wgMediaViewerUseThumbnailGuessing, $wgMediaViewerShowSurvey; + $wgMediaViewerIsInBeta, $wgMediaViewerUseThumbnailGuessing; $vars['wgMultimediaViewer'] = array( 'infoLink' => self::$infoLink, 'discussionLink' => self::$discussionLink, 'helpLink' => self::$helpLink, 'globalUsageAvailable' => isset( $wgAPIPropModules['globalusage'] ), 'useThumbnailGuessing' => (bool)$wgMediaViewerUseThumbnailGuessing, - 'showSurvey' => (bool)$wgMediaViewerShowSurvey, 'durationSamplingFactor' => $wgMediaViewerDurationLoggingSamplingFactor, 'networkPerformanceSamplingFactor' => $wgNetworkPerformanceSamplingFactor, 'actionLoggingSamplingFactorMap' => $wgMediaViewerActionLoggingSamplingFactorMap, diff --git a/i18n/en.json b/i18n/en.json index 5bd019475..d42e399d7 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -103,7 +103,6 @@ "multimediaviewer-embed-dimensions": "$1 × $2 px", "multimediaviewer-embed-dimensions-separated": "- $1", "multimediaviewer-embed-dimensions-with-file-format": "$1 $2", - "multimediaviewer-feedback-popup-text": "Leave feedback about this new media viewing experience", "multimediaviewer-description-page-button-text": "More details about this file", "multimediaviewer-description-page-popup-text": "More details about this file on $1", "multimediaviewer-commons-subtitle": "The free media repository", diff --git a/i18n/qqq.json b/i18n/qqq.json index 4f4526f5e..8dfc2fbdb 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -109,7 +109,6 @@ "multimediaviewer-embed-dimensions": "Dimensions for a given size selector option which will generate wikitext for a thumbnail.\n* $1 - width in pixels\n* $2 - height in pixels", "multimediaviewer-embed-dimensions-separated": "Wraps the dimensions with a separator styled the same way, for the embed tab.\n* $1 - image dimensions\n\nSee also:\n* {{msg-mw|Multimediaviewer-embed-dimensions}}", "multimediaviewer-embed-dimensions-with-file-format": "Collates image dimensions and a file format.\n* $1 - {{msg-mw|Multimediaviewer-embed-dimensions}}\n* $2 - file format extension, lowercased", - "multimediaviewer-feedback-popup-text": "Additional popup text of the button on top of the metadata panel which brings up a feedback survey.", "multimediaviewer-description-page-button-text": "Text of the tooltip popup for the button on top of the metadata panel which links to the file description page. Used when the file was uploaded to the local wiki.\n\nSee also:\n* {{msg-mw|Multimediaviewer-description-page-popup-text}} (for remote files)", "multimediaviewer-description-page-popup-text": "Text of the tooltip popup for the button on top of the metadata panel, which links to the file description page.Used when the file was uploaded to a different wiki.\n\nParameters:\n* $1 - the name of the site where the file comes from (e.g. \"Wikimedia Commons\")\nSee also:\n* {{msg-mw|Multimediaviewer-description-page-button-text}} (for local files)", "multimediaviewer-commons-subtitle": "Additional text to display under {{msg-mw|multimediaviewer-repository}} when the image is from Commons.", diff --git a/resources/mmv/ui/mmv.ui.stripeButtons.js b/resources/mmv/ui/mmv.ui.stripeButtons.js index 403f385a2..dc9661ed8 100644 --- a/resources/mmv/ui/mmv.ui.stripeButtons.js +++ b/resources/mmv/ui/mmv.ui.stripeButtons.js @@ -26,14 +26,10 @@ * @constructor * @param {jQuery} $container the title block (.mw-mmv-title-contain) which wraps the buttons and all * other title elements - * @param {Object} localStorage the localStorage object, for dependency injection */ - function StripeButtons( $container, localStorage ) { + function StripeButtons( $container ) { mw.mmv.ui.Element.call( this, $container ); - /** @property {Object} localStorage the window.localStorage object */ - this.localStorage = localStorage; - this.$buttonContainer = $( '
' ) .addClass( 'mw-mmv-stripe-button-container' ) .appendTo( $container ); @@ -44,9 +40,6 @@ */ this.buttons = {}; - if ( this.shouldShowFeedbackSurvey() ) { - this.initFeedbackButton(); - } this.initReuseButton(); this.initDescriptionPageButton(); } @@ -111,206 +104,6 @@ } ).prependTo( this.$container ); }; - /** - * @protected - * Creates the feedback button. - */ - SBP.initFeedbackButton = function() { - var buttons = this; - - this.buttons.$feedback = this.createButton( - 'mw-mmv-stripe-button-feedback', - null, - mw.message( 'multimediaviewer-feedback-popup-text' ).plain() - ).attr( { - target: '_blank', - href: this.getFeedbackSurveyUrl() - } ).click( function ( e ) { - buttons.openSurveyInNewWindow(); - buttons.maxOutTooltipDisplayCount(); - e.preventDefault(); - } ); - }; - - SBP.feedbackSettings = { - /** Show the tooltip this many seconds to get the user's attention, even when it is not hovered. */ - tooltipDisplayDuration: 5, - /** Wait for this long after the viewer is opened, before showing the tooltip. */ - tooltipDelay: 5, - /** Only show the tooltip this many times */ - tooltipMaxDisplayCount: 3 - }; - - /** - * Returns the number of times the tooltip was shown so far. This number is set to this.feedbackSettings.tooltipMaxDisplayCount if the - * user clicked on the link already, or we cannot count how many times the tooltip was shown - * already. - * @return {number} - */ - SBP.getTooltipDisplayCount = function () { - if ( !this.localStorage ) { - return this.feedbackSettings.tooltipMaxDisplayCount; - } - if ( this.tooltipDisplayCount === undefined ) { - this.tooltipDisplayCount = this.localStorage.getItem( 'mmv.tooltipDisplayCount' ); - if ( this.tooltipDisplayCount === null ) { - this.tooltipDisplayCount = 0; - try { - this.localStorage.setItem( 'mmv.tooltipDisplayCount', 0 ); - } catch ( e ) { - // localStorage is full or disabled - this.tooltipDisplayCount = this.feedbackSettings.tooltipMaxDisplayCount; - } - } - } - return this.tooltipDisplayCount; - }; - - /** - * Increases tooltip display count. - */ - SBP.increaseTooltipDisplayCount = function () { - this.getTooltipDisplayCount(); - if ( this.tooltipDisplayCount !== undefined && this.tooltipDisplayCount < this.feedbackSettings.tooltipMaxDisplayCount ) { - this.tooltipDisplayCount++; - - if ( this.localStorage ) { - try { - this.localStorage.setItem( 'mmv.tooltipDisplayCount', this.tooltipDisplayCount ); - } catch ( e ) { - // localStorage is full or disabled - } - } - } - }; - - /** - * Sets tooltip display count so large that the tooltip will never be shown again. - * We use this for users who already opened the form. - */ - SBP.maxOutTooltipDisplayCount = function () { - this.getTooltipDisplayCount(); - if ( this.tooltipDisplayCount !== undefined ) { - this.tooltipDisplayCount = this.feedbackSettings.tooltipMaxDisplayCount; - - if ( this.localStorage ) { - try { - this.localStorage.setItem( 'mmv.tooltipDisplayCount', this.tooltipDisplayCount ); - } catch ( e ) { - // localStorage is full or disabled - } - } - } - }; - - /** - * Show the tooltip to the user if it was not shown often enough yet. - */ - SBP.maybeDisplayTooltip = function () { - if ( - this.readyToShowFeedbackTooltip && - this.getTooltipDisplayCount() < this.feedbackSettings.tooltipMaxDisplayCount - ) { - this.buttons.$feedback.tipsy( 'show' ); - this.setTimer( 'feedbackTooltip.hide', function () { - this.buttons.$feedback.tipsy( 'hide' ); - }, this.feedbackSettings.tooltipDisplayDuration * 1000 ); - this.increaseTooltipDisplayCount(); - this.readyToShowFeedbackTooltip = false; - } else { - // if the tooltip is visible already, make sure it is not hidden too quickly - this.resetTimer( 'feedbackTooltip.hide' ); - } - }; - - /** - * Returns a link to a survey in the given language or null if that language is not supported - * @param {string|null} langcode - */ - SBP.getFeedbackSurveyBaseUrlForLanguage = function ( langcode ) { - var baseLangcode, - baseUrl = 'https://www.surveymonkey.com/s/media-viewer-1', - surveyTranslations = { ca: 'ca', hu: 'hu', fr: 'fr', pt: 'pt-br', 'pt-br': 'pt-br', - de: 'de', es: 'es', nl: 'nl' }; - - baseLangcode = langcode.split( /[_-]/ )[0]; // get rid of variants - if ( baseLangcode === 'en') { - return baseUrl; - } else if ( surveyTranslations[langcode] ) { - return baseUrl + '-' + surveyTranslations[langcode]; - } else if ( surveyTranslations[baseLangcode] ) { - return baseUrl + '-' + surveyTranslations[baseLangcode]; - } else { - return null; - } - }; - - /** - * Returns a link to a survey in the user language, or null if not supported - */ - SBP.getFeedbackSurveyBaseUrl = function () { - return this.getFeedbackSurveyBaseUrlForLanguage( mw.config.get( 'wgUserLanguage' ) ); - }; - - /** - * Checks if it is suitable to show a survey to the current user. - */ - SBP.shouldShowFeedbackSurvey = function () { - return mw.config.get( 'wgMultimediaViewer' ).showSurvey && - this.getFeedbackSurveyBaseUrl(); - }; - - /** - * Generates a survey URL (currently constant but the possibility of splitting by - * editor cohort was mentioned). - * @return {string} - */ - SBP.getFeedbackSurveyUrl = function () { - return this.getFeedbackSurveyBaseUrl() + '?c=' + mw.config.get( 'wgDBname' ); - }; - - /** - * Opens the survey in a new window, or brings it up if it is already opened. - */ - SBP.openSurveyInNewWindow = function () { - var surveyWindowWidth = screen.width * 0.85, - surveyWindowHeight = screen.height * 0.85, - feedbackSurveyWindowProperties = { - left: ( screen.width - surveyWindowWidth ) / 2, - top: ( screen.height - surveyWindowHeight ) / 2, - width: surveyWindowWidth, - height: surveyWindowHeight, - menubar: 0, - toolbar: 0, - location: 0, - personalbar: 0, - status: 0, - scrollbars: 1 - }; - - if ( !this.surveyWindow || this.surveyWindow.closed ) { - this.surveyWindow = window.open( this.getFeedbackSurveyUrl(), 'mmv-survey', - this.createWindowOpenPropertyString( feedbackSurveyWindowProperties ) ); - } else { - this.surveyWindow.focus(); - } - }; - - /** - * @protected - * Takes a property object and turns it into a string suitable for the last parameter - * of window.open. - * @param {Object} properties - * @return {string} - */ - SBP.createWindowOpenPropertyString = function ( properties ) { - var propertyArray = []; - $.each( properties, function ( key, value ) { - propertyArray.push( key + '=' + value ); - } ); - return propertyArray.join( ',' ); - }; - /** * @protected * Runs code for each button, similarly to $.each. @@ -334,10 +127,6 @@ } ); this.setDescriptionPageButton( imageInfo, repoInfo ); - - if ( this.shouldShowFeedbackSurvey() ) { - this.maybeDisplayTooltip(); - } }; /** @@ -411,14 +200,6 @@ this.handleEvent( 'mmv-reuse-closed', function () { buttons.$reuse.removeClass( 'open' ); } ); - - if ( this.shouldShowFeedbackSurvey() ) { - this.readyToShowFeedbackTooltip = false; - this.setTimer( 'feedbackTooltip.show', function () { - this.readyToShowFeedbackTooltip = true; - this.maybeDisplayTooltip(); - }, this.feedbackSettings.tooltipDelay * 1000 ); - } }; /** diff --git a/resources/mmv/ui/mmv.ui.stripeButtons.less b/resources/mmv/ui/mmv.ui.stripeButtons.less index 082299bb9..da8d8db2a 100644 --- a/resources/mmv/ui/mmv.ui.stripeButtons.less +++ b/resources/mmv/ui/mmv.ui.stripeButtons.less @@ -64,11 +64,6 @@ /* @embed */ background-image: url(img/use-ltr.svg); } - &-feedback:before { - /* @embed */ - background-image: url(img/horn_grey.svg); - background-size: 100%; - } } .mw-mmv-title-contain { diff --git a/tests/qunit/mmv/ui/mmv.ui.stripeButtons.test.js b/tests/qunit/mmv/ui/mmv.ui.stripeButtons.test.js index 61731194b..7c3a59282 100644 --- a/tests/qunit/mmv/ui/mmv.ui.stripeButtons.test.js +++ b/tests/qunit/mmv/ui/mmv.ui.stripeButtons.test.js @@ -16,26 +16,11 @@ */ ( function( mw, $ ) { - var oldShowSurvey; - - QUnit.module( 'mmv.ui.StripeButtons', QUnit.newMwEnvironment( { - setup: function () { - // pretend surveys are enabled for this site - oldShowSurvey = mw.config.get( 'wgMultimediaViewer' ).showSurvey; - mw.config.get( 'wgMultimediaViewer' ).showSurvey = true; - this.clock = this.sandbox.useFakeTimers(); - }, - teardown: function () { - mw.config.get( 'wgMultimediaViewer' ).showSurvey = oldShowSurvey; - } - } ) ); + QUnit.module( 'mmv.ui.StripeButtons', QUnit.newMwEnvironment() ); function createStripeButtons() { var fixture = $( '#qunit-fixture' ); - return new mw.mmv.ui.StripeButtons( fixture, { - getItem: function () { return mw.mmv.ui.StripeButtons.feedbackSettings.tooltipMaxDisplayCount; }, - setItem: $.noop - } ); + return new mw.mmv.ui.StripeButtons( fixture ); } QUnit.test( 'Sanity test, object creation and UI construction', 4, function ( assert ) { @@ -59,48 +44,6 @@ mw.user.isAnon = oldMwUserIsAnon; } ); - QUnit.test( 'Survey conditions', 3, function ( assert ) { - var buttons, - oldLanguage = mw.config.get( 'wgUserLanguage' ); - - // pretend surveys are disabled for this site - mw.config.get( 'wgMultimediaViewer' ).showSurvey = false; - mw.config.set( 'wgUserLanguage', 'en' ); - buttons = createStripeButtons(); - assert.ok( !buttons.buttons.$feedback, 'No survey button by default.' ); - - // pretend surveys are enabled for this site - mw.config.get( 'wgMultimediaViewer' ).showSurvey = true; - buttons = createStripeButtons(); - assert.ok( buttons.buttons.$feedback, 'Survey button shown when enabled.' ); - - // now pretend we don't speak English - mw.config.set( 'wgUserLanguage', 'el' ); - buttons = createStripeButtons(); - assert.ok( !buttons.buttons.$feedback, 'No survey for non-english speakers.' ); - - mw.config.set( 'wgUserLanguage', oldLanguage ); - } ); - - QUnit.test( 'getFeedbackSurveyBaseUrlForLanguage()', 7, function ( assert ) { - var buttons = createStripeButtons(); - - assert.strictEqual( buttons.getFeedbackSurveyBaseUrlForLanguage( 'en' ), - 'https://www.surveymonkey.com/s/media-viewer-1', 'Base survey URL for english' ); - assert.strictEqual( buttons.getFeedbackSurveyBaseUrlForLanguage( 'hu' ), - 'https://www.surveymonkey.com/s/media-viewer-1-hu', 'Language code appended for supported languages' ); - assert.strictEqual( buttons.getFeedbackSurveyBaseUrlForLanguage( 'el' ), - null, 'Null for non-supported languages' ); - assert.strictEqual( buttons.getFeedbackSurveyBaseUrlForLanguage( 'en-gb' ), - 'https://www.surveymonkey.com/s/media-viewer-1', 'Base survey URL for english variants' ); - assert.strictEqual( buttons.getFeedbackSurveyBaseUrlForLanguage( 'fr-xx' ), - 'https://www.surveymonkey.com/s/media-viewer-1-fr', 'Base code appended for other variants' ); - assert.strictEqual( buttons.getFeedbackSurveyBaseUrlForLanguage( 'pt-br' ), - 'https://www.surveymonkey.com/s/media-viewer-1-pt-br', 'Full code appended if the variant itself is supported' ); - assert.strictEqual( buttons.getFeedbackSurveyBaseUrlForLanguage( 'pt' ), - 'https://www.surveymonkey.com/s/media-viewer-1-pt-br', 'Temporary special case for pt' ); - } ); - QUnit.test( 'set()/empty() sanity test:', 1, function ( assert ) { var buttons = createStripeButtons(), fakeImageInfo = { descriptionUrl: '//commons.wikimedia.org/wiki/File:Foo.jpg' }, @@ -133,89 +76,4 @@ $( document ).off( 'mmv-reuse-open.test' ); } ); - - QUnit.test( 'Feedback tooltip', 8, function ( assert ) { - var buttons = createStripeButtons(), - displayCount, - hasTooltip = function () { return !!$( '.tipsy' ).length; }; - - this.sandbox.stub( buttons.localStorage, 'getItem', function () { return displayCount; } ); - this.sandbox.stub( buttons.localStorage, 'setItem', function ( _, val ) { displayCount = val; } ); - - displayCount = 0; - buttons.attach(); - - assert.ok( !hasTooltip(), 'No tooltip initially' ); - - this.clock.tick( 1000 ); - assert.ok( !hasTooltip(), 'No tooltip after 1s' ); - - this.clock.tick( 5000 ); - assert.ok( hasTooltip(), 'Tooltip visible after 6s' ); - assert.strictEqual( displayCount, 1, 'displayCount was increased' ); - - this.clock.tick( 5000 ); - assert.ok( !hasTooltip(), 'Tooltip hidden again after 11s' ); - - buttons.unattach(); - delete buttons.tooltipDisplayCount; - - displayCount = 3; - buttons.attach(); - - this.clock.tick( 6000 ); - assert.ok( !hasTooltip(), 'No tooltip after 6s when display count limit reached' ); - - buttons.unattach(); - delete buttons.tooltipDisplayCount; - - displayCount = 0; - buttons.openSurveyInNewWindow = this.sandbox.stub(); - buttons.attach(); - buttons.buttons.$feedback.triggerHandler( 'click' ); - - this.clock.tick( 6000 ); - assert.ok( !hasTooltip(), 'No tooltip if button was clicked' ); - - buttons.unattach(); - delete buttons.tooltipDisplayCount; - - displayCount = 0; - buttons.attach(); - buttons.unattach(); - this.clock.tick( 6000 ); - assert.ok( !hasTooltip(), 'No tooltip when unattached' ); - } ); - - QUnit.test( 'No localStorage', 3, function( assert ) { - var fixture = $( '#qunit-fixture' ), - buttons = new mw.mmv.ui.StripeButtons( fixture, undefined ); - - assert.strictEqual( buttons.getTooltipDisplayCount(), buttons.feedbackSettings.tooltipMaxDisplayCount, 'Initial tooltip count is tooltipMaxDisplayCount' ); - - buttons.increaseTooltipDisplayCount(); - - assert.strictEqual( buttons.getTooltipDisplayCount(), buttons.feedbackSettings.tooltipMaxDisplayCount, 'Tooltip count is still tooltipMaxDisplayCount' ); - - buttons.maxOutTooltipDisplayCount(); - - assert.strictEqual( buttons.getTooltipDisplayCount(), buttons.feedbackSettings.tooltipMaxDisplayCount, 'Tooltip count is still tooltipMaxDisplayCount' ); - } ); - - QUnit.test( 'Full localStorage', 3, function( assert ) { - var buttons = createStripeButtons(); - - this.sandbox.stub( buttons.localStorage, 'getItem', function () { return null; } ); - this.sandbox.stub( buttons.localStorage, 'setItem', function () { throw 'I am full'; } ); - - assert.strictEqual( buttons.getTooltipDisplayCount(), buttons.feedbackSettings.tooltipMaxDisplayCount, 'Initial tooltip count is tooltipMaxDisplayCount' ); - - buttons.increaseTooltipDisplayCount(); - - assert.strictEqual( buttons.getTooltipDisplayCount(), buttons.feedbackSettings.tooltipMaxDisplayCount, 'Tooltip count is still tooltipMaxDisplayCount' ); - - buttons.maxOutTooltipDisplayCount(); - - assert.strictEqual( buttons.getTooltipDisplayCount(), buttons.feedbackSettings.tooltipMaxDisplayCount, 'Tooltip count is still tooltipMaxDisplayCount' ); - } ); }( mediaWiki, jQuery ) );