From 62c1d64ad082d93a35273da0c5ec2033e61f9713 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Wed, 4 Feb 2015 17:45:19 -0800 Subject: [PATCH] mw.ViewPageTarget: Use CSS instead of JS for DOM hiding/muting * Use .ve-activated for elements changed in activate() - hideReadOnlyContent() - mutePageContent() - mutePageTitle * Use .ve-active for elements changed in onSurfaceReady() - hidePageContent() * Set 've-activated' class from activate() instead of transformPage() to consolidate reflows and minimise DOM interaction. It's still in the same (synchronous) execution path, but a few statements earlier now. * Remove obsolete #toc wrapper. This
(with data property to distinguish it from potentially foreign parents in the future) was there to aid slideDown/slideUp animations, because those don't work well on table elements. See eba7d58dd19d9. * Remove obsolete setTimeout in restorePageTitle(). The removal of .ve-init-mw-viewPageTarget-pageTitle was delayed by one second (introduced in Ibc3fa2fb7 / 4cc88b9850). This was to account for a jQuery animation we no longer use. * Remove unused '.ve-init-mw-viewPageTarget-transform-muted' * Remove unused '.ve-init-mw-viewPageTarget-transform' * Remove unused '.ve-init-mw-viewPageTarget-pageTitle' The resulting stylesheet exposes that we're not consistent in whether elements hide immediately (ve-activated) or once the surface is ready (ve-active). This is intentionally kept as-is within this commit. Of the different elements that had their opacity changed, only firstHeading was being animated. This animation was removed. Bug: T88590 Bug: T87160 Change-Id: I87033456f715d99a88425e38e8ac5171144f4ec8 --- .../init/styles/ve.init.mw.ViewPageTarget.css | 32 +++-- .../init/targets/ve.init.mw.ViewPageTarget.js | 121 ++---------------- 2 files changed, 32 insertions(+), 121 deletions(-) diff --git a/modules/ve-mw/init/styles/ve.init.mw.ViewPageTarget.css b/modules/ve-mw/init/styles/ve.init.mw.ViewPageTarget.css index 3f71eaf314..7866f474d6 100644 --- a/modules/ve-mw/init/styles/ve.init.mw.ViewPageTarget.css +++ b/modules/ve-mw/init/styles/ve.init.mw.ViewPageTarget.css @@ -5,20 +5,36 @@ * @license The MIT License (MIT); see LICENSE.txt */ -/* VisualEditor */ +/*! + * State | classes + * Reading | ve-available + * Activate editor | ve-available ve-activated ve-activating + * Active | ve-available ve-activated ve-active + * Deactivate editor | ve-available ve-deactivating + * Deactivated | ve-available + */ -.ve-init-mw-viewPageTarget-transform { - -webkit-transition: opacity 200ms ease-out; - -moz-transition: opacity 200ms ease-out; - -o-transition: opacity 200ms ease-out; - transition: opacity 200ms ease-out; +.ve-activated #contentSub, +.ve-activated #toc, +/* Most of bodyContent can be hidden as VE has an equivalent of most children + in ve-init-target (sibling of #bodyContent). However, we can't hide it + completely as #siteSub should remain visible (for persistence with read mode), + and ve-ui-mwTocWidget is also part of #bodyContent. */ +.ve-active #bodyContent > :not(#siteSub):not(.ve-ui-mwTocWidget), +.ve-active #t-print, +.ve-active #t-permalink, +.ve-active #p-coll-print_export, +.ve-active #t-cite { + display: none; } -.ve-init-mw-viewPageTarget-transform-muted { +.ve-activating #bodyContent, +.ve-activated #firstHeading, +.ve-activated #siteSub { opacity: 0.6; } -.ve-init-mw-viewPageTarget-pageTitle { +.ve-activated #firstHeading { cursor: default; } diff --git a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js index e4dae21992..cec0dd199f 100644 --- a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js +++ b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js @@ -276,6 +276,11 @@ ve.init.mw.ViewPageTarget.prototype.activate = function () { this.activating = true; this.activatingDeferred = $.Deferred(); + $( 'html' ).addClass( 've-activating ve-activated' ); + this.activatingDeferred.always( function () { + $( 'html' ).addClass( 've-active' ).removeClass( 've-activating' ); + } ); + this.bindHandlers(); this.originalEditondbclick = mw.user.options.get( 'editondblclick' ); @@ -283,9 +288,6 @@ ve.init.mw.ViewPageTarget.prototype.activate = function () { // User interface changes this.transformPage(); - this.hideReadOnlyContent(); - this.mutePageContent(); - this.mutePageTitle(); this.setupLocalNoticeMessages(); this.saveScrollPosition(); @@ -357,12 +359,12 @@ ve.init.mw.ViewPageTarget.prototype.cancel = function ( trackMechanism ) { } this.deactivating = true; + $( 'html' ).removeClass( 've-activated' ).addClass( 've-deactivating' ); // User interface changes if ( this.elementsThatHadOurAccessKey ) { this.elementsThatHadOurAccessKey.attr( 'accesskey', ve.msg( 'accesskey-save' ) ); } this.restorePage(); - this.showReadOnlyContent(); this.unbindHandlers(); @@ -386,10 +388,6 @@ ve.init.mw.ViewPageTarget.prototype.cancel = function ( trackMechanism ) { } $.when.apply( null, promises ).done( function () { - // Show/restore components that are otherwise handled by tearDownSurface - target.showPageContent(); - target.restorePageTitle(); - // If there is a load in progress, abort it if ( target.loading ) { target.loading.abort(); @@ -402,6 +400,7 @@ ve.init.mw.ViewPageTarget.prototype.cancel = function ( trackMechanism ) { target.deactivating = false; target.activating = false; target.activatingDeferred.reject(); + $( 'html' ).removeClass( 've-active ve-deactivating' ); mw.hook( 've.deactivationComplete' ).fire( target.edited ); } ); @@ -456,6 +455,7 @@ ve.init.mw.ViewPageTarget.prototype.onSurfaceReady = function () { // TODO are there things we need to clean up? return; } + this.activating = false; this.getSurface().getModel().connect( this, { history: 'updateToolbarSaveButtonState' @@ -472,9 +472,7 @@ ve.init.mw.ViewPageTarget.prototype.onSurfaceReady = function () { } ); // Update UI - this.transformPageTitle(); this.changeDocumentTitle(); - this.hidePageContent(); this.getSurface().getView().focus(); @@ -1273,75 +1271,6 @@ ve.init.mw.ViewPageTarget.prototype.restoreScrollPosition = function () { } }; -/** - * Show the page content. - * - * @method - */ -ve.init.mw.ViewPageTarget.prototype.showPageContent = function () { - $( '#bodyContent > .ve-init-mw-viewPageTarget-content' ) - .removeClass( 've-init-mw-viewPageTarget-content' ) - .show() - .fadeTo( 0, 1 ); - $( '#t-print, #t-permalink, #p-coll-print_export, #t-cite' ).show(); -}; - -/** - * Mute the page content. - * - * @method - */ -ve.init.mw.ViewPageTarget.prototype.mutePageContent = function () { - $( '#bodyContent > :visible:not(#siteSub)' ) - .addClass( 've-init-mw-viewPageTarget-content' ) - .fadeTo( 'fast', 0.6 ); -}; - -/** - * Hide the page content. - * - * @method - */ -ve.init.mw.ViewPageTarget.prototype.hidePageContent = function () { - $( '#bodyContent > :visible:not(#siteSub,.ve-ui-mwTocWidget)' ) - .addClass( 've-init-mw-viewPageTarget-content' ) - .hide(); - $( '#t-print, #t-permalink, #p-coll-print_export, #t-cite' ).hide(); -}; - -/** - * Show elements that didn't have a counter-part in the edit view. - */ -ve.init.mw.ViewPageTarget.prototype.showReadOnlyContent = function () { - var $toc = $( '#toc' ), - $wrap = $toc.parent(); - if ( $wrap.data( 've.hideTableOfContents' ) ) { - $wrap.show( function () { - $toc.unwrap(); - } ); - } - - $( '#contentSub' ).show(); -}; - -/** - * Hide elements that don't have a counter-part in the edit view. - * - * Call this when puting the page content, so that when we replace the - * muted content with the edit surface, everything aligns in the same - * place. If things like contentSub and TOC remain visible in mute mode, - * there is an additional visual shift that is unpleasant to the user. - */ -ve.init.mw.ViewPageTarget.prototype.hideReadOnlyContent = function () { - $( '#toc' ) - .wrap( '
' ) - .parent() - .data( 've.hideTableOfContents', true ) - .hide(); - - $( '#contentSub' ).hide(); -}; - /** * Hide the toolbar. */ @@ -1366,34 +1295,6 @@ ve.init.mw.ViewPageTarget.prototype.tearDownDebugBar = function () { } }; -/** - * Transform the page title into a VE-style title. - */ -ve.init.mw.ViewPageTarget.prototype.transformPageTitle = function () { - $( '#firstHeading' ).addClass( 've-init-mw-viewPageTarget-pageTitle' ); -}; - -/** - * Fade the page title to indicate it is not editable. - */ -ve.init.mw.ViewPageTarget.prototype.mutePageTitle = function () { - $( '#firstHeading, #siteSub' ) - .addClass( 've-init-mw-viewPageTarget-transform ve-init-mw-viewPageTarget-transform-muted' ); -}; - -/** - * Restore the page title to its original style. - */ -ve.init.mw.ViewPageTarget.prototype.restorePageTitle = function () { - var $els = $( '#firstHeading, #siteSub' ) - .removeClass( 've-init-mw-viewPageTarget-transform-muted' ); - - setTimeout( function () { - $els.removeClass( 've-init-mw-viewPageTarget-transform' ); - $( '#firstHeading' ).removeClass( 've-init-mw-viewPageTarget-pageTitle' ); - }, 1000 ); -}; - /** * Change the document title to state that we are now editing. */ @@ -1433,9 +1334,6 @@ ve.init.mw.ViewPageTarget.prototype.transformPage = function () { .addClass( 've-hide' ) .fadeOut( 'fast' ); - // Add class to document - $( 'html' ).addClass( 've-activated' ); - // Push veaction=edit url in history (if not already. If we got here by a veaction=edit // permalink then it will be there already and the constructor called #activate) if ( !this.actFromPopState && history.pushState && this.currentUri.query.veaction !== 'edit' ) { @@ -1469,9 +1367,6 @@ ve.init.mw.ViewPageTarget.prototype.restorePage = function () { $( '.mw-indicators.ve-hide' ) .fadeIn( 'fast' ); - // Remove class from document - $( 'html' ).removeClass( 've-activated' ); - // Push non-veaction=edit url in history if ( !this.actFromPopState && history.pushState ) { // Remove the veaction query parameter