jenkins-bot has submitted this change and it was merged.
Change subject: FocusableNode: watch for image loaded/layout changes for
invisible icons
......................................................................
FocusableNode: watch for image loaded/layout changes for invisible icons
jQuery width / height is a bit rendering-dependent, and will return 0 if
called particularly early. This results in some cases like the Math extension
appearing to not have rendering, because they just contain images.
Solution: We can check on the raw DOM properties for elements which have them,
which work before layout occurs. We can also watch for a load event on images
in the node, and redo the rendering.
Also, expand the documentation for hasRendering a bit, so it's clearer what we
consider to be "visible".
Bug: T125767
Change-Id: If322b2cde668e4ca774355fd7c02152f05ce67fa
---
M src/ce/ve.ce.FocusableNode.js
1 file changed, 53 insertions(+), 17 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/ce/ve.ce.FocusableNode.js b/src/ce/ve.ce.FocusableNode.js
index 4117490..97b2878 100644
--- a/src/ce/ve.ce.FocusableNode.js
+++ b/src/ce/ve.ce.FocusableNode.js
@@ -146,25 +146,48 @@
}
if ( this.constructor.static.iconWhenInvisible ) {
- if ( !this.hasRendering() ) {
- if ( !this.icon ) {
- this.icon = new OO.ui.IconWidget( {
- classes: [
've-ce-focusableNode-invisibleIcon' ],
- icon:
this.constructor.static.iconWhenInvisible
- } );
- // Add em space for selection highlighting
- this.icon.$element.text( '\u2003' );
- }
- this.$element.first()
- .addClass( 've-ce-focusableNode-invisible' )
- .prepend( this.icon.$element );
- } else if ( this.icon ) {
- this.$element.first().removeClass(
've-ce-focusableNode-invisible' );
- this.icon.$element.detach();
- }
+ // Set up the invisible icon, and watch for its continued
necessity if
+ // unloaded images which don't specify their width or height are
+ // involved.
+ this.$element
+ .find( 'img:not([width]),img:not([height])' )
+ .addBack( 'img:not([width]),img:not([height])' )
+ .on( 'load', this.updateInvisibleIcon.bind( this ) );
+ this.updateInvisibleIcon();
}
this.isFocusableSetup = true;
+};
+
+/**
+ * Update the state of icon if this node is invisible
+ *
+ * If the node doesn't have a visible rendering, we insert an icon to represent
+ * it. If the icon was already present, and this is called again when rendering
+ * has developed, we remove the icon.
+ *
+ * @method
+ */
+ve.ce.FocusableNode.prototype.updateInvisibleIcon = function () {
+ if ( !this.constructor.static.iconWhenInvisible ) {
+ return;
+ }
+ if ( !this.hasRendering() ) {
+ if ( !this.icon ) {
+ this.icon = new OO.ui.IconWidget( {
+ classes: [ 've-ce-focusableNode-invisibleIcon'
],
+ icon: this.constructor.static.iconWhenInvisible
+ } );
+ // Add em space for selection highlighting
+ this.icon.$element.text( '\u2003' );
+ }
+ this.$element.first()
+ .addClass( 've-ce-focusableNode-invisible' )
+ .prepend( this.icon.$element );
+ } else if ( this.icon ) {
+ this.$element.first().removeClass(
've-ce-focusableNode-invisible' );
+ this.icon.$element.detach();
+ }
};
/**
@@ -661,6 +684,10 @@
/**
* Check if the rendering is visible
*
+ * "Visible", in this case, is defined as any of:
+ * * contains any non-whitespace text
+ * * is greater than 8px x 8px in dimensions
+ *
* @return {boolean} The node has a visible rendering
*/
ve.ce.FocusableNode.prototype.hasRendering = function () {
@@ -670,7 +697,16 @@
}
this.$element.each( function () {
var $this = $( this );
- if ( $this.width() >= 8 && $this.height() >= 8 ) {
+ if (
+ ( $this.width() >= 8 && $this.height() >= 8 ) ||
+ // jQuery handles disparate cases, but is prone to
elements which
+ // haven't experienced layout yet having 0 width /
height. So,
+ // check the raw DOM width / height properties as well.
If it's an
+ // image or other thing-with-width, this will work
slightly more
+ // reliably. If it's not, this will be undefined and the
+ // comparison will thus just be false.
+ ( this.width >= 8 && this.height >= 8 )
+ ) {
visible = true;
return false;
}
--
To view, visit https://gerrit.wikimedia.org/r/268437
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If322b2cde668e4ca774355fd7c02152f05ce67fa
Gerrit-PatchSet: 7
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: DLynch <[email protected]>
Gerrit-Reviewer: DLynch <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
