jenkins-bot has submitted this change and it was merged.
Change subject: Don't call .addClass() in CE nodes' setup handlers
......................................................................
Don't call .addClass() in CE nodes' setup handlers
Adding CSS classes on setup causes nodes to first render without
the class, then rerender with the class. This is wasteful. Instead,
add classes in the constructor.
In LeafNode, BranchNode and FocusableNode we also set classes
on setup, because GeneratedContentNode does crazy things to this.$element
and it's important we don't lose these classes (a lot of code
checks for the presence of these classes).
In TableCellNode, we need to update classes when the style
attribute changes.
Change-Id: I2690751d16977b106831c94a0757b615985fa5d6
---
M src/ce/nodes/ve.ce.TableCaptionNode.js
M src/ce/nodes/ve.ce.TableCellNode.js
M src/ce/nodes/ve.ce.TableNode.js
M src/ce/ve.ce.BranchNode.js
M src/ce/ve.ce.FocusableNode.js
M src/ce/ve.ce.LeafNode.js
6 files changed, 52 insertions(+), 82 deletions(-)
Approvals:
Jforrester: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/ce/nodes/ve.ce.TableCaptionNode.js
b/src/ce/nodes/ve.ce.TableCaptionNode.js
index 37b733e..1527d81 100644
--- a/src/ce/nodes/ve.ce.TableCaptionNode.js
+++ b/src/ce/nodes/ve.ce.TableCaptionNode.js
@@ -16,6 +16,11 @@
ve.ce.TableCaptionNode = function VeCeTableCaptionNode() {
// Parent constructor
ve.ce.TableCaptionNode.super.apply( this, arguments );
+
+ // DOM changes
+ this.$element
+ .addClass( 've-ce-tableCaptionNode' )
+ .prop( 'contentEditable', 'true' );
};
/* Inheritance */
@@ -27,21 +32,6 @@
ve.ce.TableCaptionNode.static.name = 'tableCaption';
ve.ce.TableCaptionNode.static.tagName = 'caption';
-
-/* Methods */
-
-/**
- * @inheritdoc
- */
-ve.ce.TableCaptionNode.prototype.onSetup = function () {
- // Parent method
- ve.ce.TableCaptionNode.super.prototype.onSetup.call( this );
-
- // DOM changes
- this.$element
- .addClass( 've-ce-tableCaptionNode' )
- .prop( 'contentEditable', 'true' );
-};
/* Registration */
diff --git a/src/ce/nodes/ve.ce.TableCellNode.js
b/src/ce/nodes/ve.ce.TableCellNode.js
index 25f38b1..f2980ce 100644
--- a/src/ce/nodes/ve.ce.TableCellNode.js
+++ b/src/ce/nodes/ve.ce.TableCellNode.js
@@ -17,6 +17,22 @@
// Parent constructor
ve.ce.TableCellNode.super.apply( this, arguments );
+ var rowspan = this.model.getRowspan(),
+ colspan = this.model.getColspan();
+
+ // DOM changes
+ this.$element
+ // The following classes can be used here:
+ // ve-ce-tableCellNode-data
+ // ve-ce-tableCellNode-header
+ .addClass( 've-ce-tableCellNode ve-ce-tableCellNode-' +
this.model.getAttribute( 'style' ) );
+ if ( rowspan > 1 ) {
+ this.$element.attr( 'rowspan', rowspan );
+ }
+ if ( colspan > 1 ) {
+ this.$element.attr( 'colspan', colspan );
+ }
+
// Events
this.model.connect( this, {
update: 'onUpdate',
@@ -33,46 +49,6 @@
ve.ce.TableCellNode.static.name = 'tableCell';
/* Methods */
-
-/**
- * @inheritdoc
- */
-ve.ce.TableCellNode.prototype.onSetup = function () {
- var rowspan = this.model.getRowspan(),
- colspan = this.model.getColspan();
-
- // Parent method
- ve.ce.TableCellNode.super.prototype.onSetup.call( this );
-
- // Exit if already setup or not attached
- if ( this.isSetup || !this.root ) {
- return;
- }
-
- // DOM changes
- this.$element
- // The following classes can be used here:
- // ve-ce-tableCellNode-data
- // ve-ce-tableCellNode-header
- .addClass( 've-ce-tableCellNode ve-ce-tableCellNode-' +
this.model.getAttribute( 'style' ) );
-
- if ( rowspan > 1 ) {
- this.$element.attr( 'rowspan', rowspan );
- }
- if ( colspan > 1 ) {
- this.$element.attr( 'colspan', colspan );
- }
-};
-
-/**
- * @inheritdoc
- */
-ve.ce.TableCellNode.prototype.onTeardown = function () {
- // Parent method
- ve.ce.TableCellNode.super.prototype.onTeardown.call( this );
-
- this.$element.removeClass( 've-ce-tableCellNode
ve-ce-tableCellNode-data ve-ce-tableCellNode-header' );
-};
/**
* Get the HTML tag name.
@@ -128,6 +104,9 @@
}
break;
case 'style':
+ this.$element
+ .removeClass( 've-ce-tableCellNode-' + from )
+ .addClass( 've-ce-tableCellNode-' + to );
this.updateTagName();
break;
}
diff --git a/src/ce/nodes/ve.ce.TableNode.js b/src/ce/nodes/ve.ce.TableNode.js
index e110f20..ac12548 100644
--- a/src/ce/nodes/ve.ce.TableNode.js
+++ b/src/ce/nodes/ve.ce.TableNode.js
@@ -17,10 +17,16 @@
// Parent constructor
ve.ce.TableNode.super.apply( this, arguments );
+ // Properties
this.surface = null;
this.active = false;
this.startCell = null;
this.editingFragment = null;
+
+ // DOM changes
+ this.$element
+ .addClass( 've-ce-tableNode' )
+ .prop( 'contentEditable', 'false' );
};
/* Inheritance */
@@ -41,11 +47,6 @@
return;
}
this.surface = this.getRoot().getSurface();
-
- // DOM changes
- this.$element
- .addClass( 've-ce-tableNode' )
- .prop( 'contentEditable', 'false' );
// Overlay
this.$selectionBox = this.$( '<div>' ).addClass(
've-ce-tableNodeOverlay-selection-box' );
diff --git a/src/ce/ve.ce.BranchNode.js b/src/ce/ve.ce.BranchNode.js
index 3e3ceca..a4513b5 100644
--- a/src/ce/ve.ce.BranchNode.js
+++ b/src/ce/ve.ce.BranchNode.js
@@ -24,6 +24,9 @@
// Parent constructor
ve.ce.Node.call( this, model, config );
+ // DOM changes (keep in sync with #onSetup)
+ this.$element.addClass( 've-ce-branchNode' );
+
// Properties
this.tagName = this.$element.get( 0 ).nodeName.toLowerCase();
this.slugNodes = [];
@@ -95,23 +98,14 @@
/* Methods */
/**
- * Handle setup event.
- *
- * @method
+ * @inheritdoc
*/
ve.ce.BranchNode.prototype.onSetup = function () {
+ // Parent method
ve.ce.Node.prototype.onSetup.call( this );
- this.$element.addClass( 've-ce-branchNode' );
-};
-/**
- * Handle teardown event.
- *
- * @method
- */
-ve.ce.BranchNode.prototype.onTeardown = function () {
- ve.ce.Node.prototype.onTeardown.call( this );
- this.$element.removeClass( 've-ce-branchNode' );
+ // DOM changes (duplicated from constructor in case this.$element is
replaced)
+ this.$element.addClass( 've-ce-branchNode' );
};
/**
diff --git a/src/ce/ve.ce.FocusableNode.js b/src/ce/ve.ce.FocusableNode.js
index 1259ce8..42d857c 100644
--- a/src/ce/ve.ce.FocusableNode.js
+++ b/src/ce/ve.ce.FocusableNode.js
@@ -35,6 +35,11 @@
this.boundingRect = null;
this.startAndEndRects = null;
+ // DOM changes
+ this.$element
+ .addClass( 've-ce-focusableNode' )
+ .prop( 'contentEditable', 'false' );
+
// Events
this.connect( this, {
setup: 'onFocusableSetup',
@@ -96,7 +101,7 @@
this.surface = this.getRoot().getSurface();
- // DOM changes
+ // DOM changes (duplicated from constructor in case this.$element is
replaced)
this.$element
.addClass( 've-ce-focusableNode' )
.prop( 'contentEditable', 'false' );
diff --git a/src/ce/ve.ce.LeafNode.js b/src/ce/ve.ce.LeafNode.js
index 4948221..992f7e3 100644
--- a/src/ce/ve.ce.LeafNode.js
+++ b/src/ce/ve.ce.LeafNode.js
@@ -24,7 +24,7 @@
// Parent constructor
ve.ce.Node.apply( this, arguments );
- // DOM changes
+ // DOM changes (keep in sync with #onSetup)
if ( model.isWrapped() ) {
this.$element.addClass( 've-ce-leafNode' );
}
@@ -42,16 +42,17 @@
/* Methods */
-/** */
+/**
+ * @inheritdoc
+ */
ve.ce.LeafNode.prototype.onSetup = function () {
+ // Parent method
ve.ce.Node.prototype.onSetup.call( this );
- this.$element.addClass( 've-ce-leafNode' );
-};
-/** */
-ve.ce.LeafNode.prototype.onTeardown = function () {
- ve.ce.Node.prototype.onTeardown.call( this );
- this.$element.removeClass( 've-ce-leafNode' );
+ // DOM changes (duplicated from constructor in case this.$element is
replaced)
+ if ( this.model.isWrapped() ) {
+ this.$element.addClass( 've-ce-leafNode' );
+ }
};
/**
--
To view, visit https://gerrit.wikimedia.org/r/193813
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2690751d16977b106831c94a0757b615985fa5d6
Gerrit-PatchSet: 6
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Ori.livneh <o...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
