Parcourir la source

Merge pull request #280 from ckeditor/t/142

Fix: Button tooltip should not look blurry on ldpi screens. Closes #142. Closes #133.
Oskar Wróbel il y a 8 ans
Parent
commit
200b0dc5d6

+ 34 - 8
packages/ckeditor5-ui/src/button/buttonview.js

@@ -10,6 +10,7 @@
 import View from '../view';
 import Template from '../template';
 import IconView from '../icon/iconview';
+import TooltipView from '../tooltip/tooltipview';
 
 import { getEnvKeystrokeText } from '@ckeditor/ckeditor5-utils/src/keyboard';
 
@@ -59,6 +60,18 @@ export default class ButtonView extends View {
 		 */
 		this.set( 'tooltip' );
 
+		/**
+		 * The position of the tooltip. See {@link ui/tooltip/tooltipview~TooltipView#position}
+		 * to learn more about the available position values.
+		 *
+		 * **Note:** It makes sense only when the {@link #tooltip} is active.
+		 *
+		 * @observable
+		 * @default 's'
+		 * @member {'s'|'n'} #position
+		 */
+		this.set( 'tooltipPosition', 's' );
+
 		/**
 		 * The HTML type of the button. Default `button`.
 		 *
@@ -140,6 +153,13 @@ export default class ButtonView extends View {
 		 * @member {module:ui/icon/iconview~IconView} #iconView
 		 */
 
+		/**
+		 * Tooltip of the button view.
+		 *
+		 * @readonly
+		 * @member {module:ui/tooltip/tooltipview~TooltipView} #tooltipView
+		 */
+
 		const bind = this.bindTemplate;
 
 		this.template = new Template( {
@@ -148,16 +168,12 @@ export default class ButtonView extends View {
 			attributes: {
 				class: [
 					'ck-button',
-					bind.if( '_tooltipString', 'ck-tooltip_s' ),
 					bind.to( 'isEnabled', value => value ? 'ck-enabled' : 'ck-disabled' ),
 					bind.if( 'isVisible', 'ck-hidden', value => !value ),
 					bind.to( 'isOn', value => value ? 'ck-on' : 'ck-off' ),
 					bind.if( 'withText', 'ck-button_with-text' )
 				],
 				type: bind.to( 'type', value => value ? value : 'button' ),
-				'data-ck-tooltip': [
-					bind.to( '_tooltipString' )
-				],
 				tabindex: bind.to( 'tabindex' )
 			},
 
@@ -207,17 +223,27 @@ export default class ButtonView extends View {
 	 * @inheritDoc
 	 */
 	init() {
-		if ( this.icon && !this.iconView ) {
+		if ( this.icon ) {
 			const iconView = this.iconView = new IconView();
 
 			iconView.bind( 'content' ).to( this, 'icon' );
-
 			this.element.insertBefore( iconView.element, this.element.firstChild );
 
-			// Make sure the icon view will be destroyed along with button.
+			// Make sure the icon will be destroyed along with the button.
 			this.addChildren( iconView );
 		}
 
+		if ( this.tooltip ) {
+			const tooltipView = this.tooltipView = new TooltipView();
+
+			tooltipView.bind( 'text' ).to( this, '_tooltipString' );
+			tooltipView.bind( 'position' ).to( this, 'tooltipPosition' );
+			this.element.appendChild( tooltipView.element );
+
+			// Make sure the tooltip will be destroyed along with the button.
+			this.addChildren( tooltipView );
+		}
+
 		super.init();
 	}
 
@@ -229,7 +255,7 @@ export default class ButtonView extends View {
 	}
 
 	/**
-	 * Gets value for the `data-ck-tooltip` attribute from the combination of
+	 * Gets the text for the {@link #tooltipView} from the combination of
 	 * {@link #tooltip}, {@link #label} and {@link #keystroke} attributes.
 	 *
 	 * @private

+ 83 - 0
packages/ckeditor5-ui/src/tooltip/tooltipview.js

@@ -0,0 +1,83 @@
+/**
+ * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/**
+ * @module ui/tooltip/tooltipview
+ */
+
+import View from '../view';
+import Template from '../template';
+
+/**
+ * The tooltip view class.
+ *
+ * @extends module:ui/view~View
+ */
+export default class TooltipView extends View {
+	/**
+	 * @inheritDoc
+	 */
+	constructor( locale ) {
+		super( locale );
+
+		/**
+		 * The text of the tooltip visible to the user.
+		 *
+		 * @observable
+		 * @member {String} #text
+		 */
+		this.set( 'text' );
+
+		/**
+		 * The position of the tooltip (south or north).
+		 *
+		 *		+-----------+
+		 *		|   north   |
+		 *		+-----------+
+		 *		      V
+		 *		  [element]
+		 *
+		 *		  [element]
+		 *		      ^
+		 *		+-----------+
+		 *		|   south   |
+		 *		+-----------+
+		 *
+		 * @observable
+		 * @default 's'
+		 * @member {'s'|'n'} #position
+		 */
+		this.set( 'position', 's' );
+
+		const bind = this.bindTemplate;
+
+		this.template = new Template( {
+			tag: 'span',
+			attributes: {
+				class: [
+					'ck-tooltip',
+					bind.to( 'position', position => 'ck-tooltip_' + position )
+				]
+			},
+			children: [
+				{
+					tag: 'span',
+
+					attributes: {
+						class: [
+							'ck-tooltip__text'
+						]
+					},
+
+					children: [
+						{
+							text: bind.to( 'text' ),
+						}
+					]
+				}
+			]
+		} );
+	}
+}

+ 63 - 15
packages/ckeditor5-ui/tests/button/buttonview.js

@@ -8,6 +8,7 @@
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import ButtonView from '../../src/button/buttonview';
 import IconView from '../../src/icon/iconview';
+import TooltipView from '../../src/tooltip/tooltipview';
 
 testUtils.createSinonSandbox();
 
@@ -75,29 +76,69 @@ describe( 'ButtonView', () => {
 		} );
 
 		describe( 'tooltip', () => {
+			beforeEach( () => {
+				view = new ButtonView( locale );
+			} );
+
 			it( 'is not initially set', () => {
-				expect( view.element.dataset.ckTooltip ).to.undefined;
+				expect( view.tooltipView ).to.be.undefined;
+				expect( view.element.childNodes ).to.have.length( 1 );
 			} );
 
 			it( 'is not initially set (despite #label and #keystroke)', () => {
 				view.label = 'foo';
 				view.keystroke = 'A';
+				view.init();
 
-				expect( view.element.dataset.ckTooltip ).to.undefined;
+				expect( view.tooltipView ).to.be.undefined;
 			} );
 
 			it( 'is not set if neither `true`, String or Function', () => {
 				view.label = 'foo';
 				view.keystroke = 'A';
 				view.tooltip = false;
+				view.init();
 
-				expect( view.element.dataset.ckTooltip ).to.undefined;
+				expect( view.tooltipView ).to.be.undefined;
 
 				view.tooltip = 3;
-				expect( view.element.dataset.ckTooltip ).to.undefined;
+				expect( view.tooltipView ).to.be.undefined;
 
 				view.tooltip = new Date();
-				expect( view.element.dataset.ckTooltip ).to.undefined;
+				expect( view.tooltipView ).to.be.undefined;
+			} );
+
+			it( 'when set, is added to the DOM', () => {
+				view.tooltip = 'foo';
+				view.icon = 'bar';
+				view.init();
+
+				expect( view.element.childNodes ).to.have.length( 3 );
+				expect( view.element.childNodes[ 2 ] ).to.equal( view.tooltipView.element );
+				expect( view.tooltipView ).to.instanceOf( TooltipView );
+				expect( view.tooltipView.position ).to.equal( 's' );
+			} );
+
+			it( 'when set, is destroyed along with the view', () => {
+				view.tooltip = 'foo';
+				view.init();
+
+				const spy = sinon.spy( view.tooltipView, 'destroy' );
+
+				view.destroy();
+				sinon.assert.calledOnce( spy );
+			} );
+
+			it( 'when set, reacts to #tooltipPosition attribute', () => {
+				view.tooltip = 'foo';
+				view.icon = 'bar';
+				view.init();
+
+				expect( view.tooltipPosition ).to.equal( 's' );
+				expect( view.tooltipView.position ).to.equal( 's' );
+
+				view.tooltipPosition = 'n';
+				expect( view.tooltipView.position ).to.equal( 'n' );
 			} );
 
 			describe( 'defined as a Boolean', () => {
@@ -105,21 +146,23 @@ describe( 'ButtonView', () => {
 					view.tooltip = true;
 					view.label = 'bar';
 					view.keystroke = 'A';
+					view.init();
 
-					expect( view.element.dataset.ckTooltip ).to.equal( 'bar (A)' );
+					expect( view.tooltipView.text ).to.equal( 'bar (A)' );
 				} );
 
 				it( 'reacts to changes in #label and #keystroke', () => {
 					view.tooltip = true;
 					view.label = 'foo';
 					view.keystroke = 'B';
+					view.init();
 
-					expect( view.element.dataset.ckTooltip ).to.equal( 'foo (B)' );
+					expect( view.tooltipView.text ).to.equal( 'foo (B)' );
 
 					view.label = 'baz';
 					view.keystroke = false;
 
-					expect( view.element.dataset.ckTooltip ).to.equal( 'baz' );
+					expect( view.tooltipView.text ).to.equal( 'baz' );
 				} );
 			} );
 
@@ -128,16 +171,19 @@ describe( 'ButtonView', () => {
 					view.tooltip = 'bar';
 					view.label = 'foo';
 					view.keystroke = 'A';
+					view.init();
 
-					expect( view.element.dataset.ckTooltip ).to.equal( 'bar' );
+					expect( view.tooltipView.text ).to.equal( 'bar' );
 				} );
 
 				it( 'reacts to changes of #tooltip', () => {
 					view.tooltip = 'bar';
-					expect( view.element.dataset.ckTooltip ).to.equal( 'bar' );
+					view.init();
+
+					expect( view.tooltipView.text ).to.equal( 'bar' );
 
 					view.tooltip = 'foo';
-					expect( view.element.dataset.ckTooltip ).to.equal( 'foo' );
+					expect( view.tooltipView.text ).to.equal( 'foo' );
 				} );
 			} );
 
@@ -146,21 +192,23 @@ describe( 'ButtonView', () => {
 					view.tooltip = ( l, k ) => `${ l } - ${ k }`;
 					view.label = 'foo';
 					view.keystroke = 'A';
+					view.init();
 
-					expect( view.element.dataset.ckTooltip ).to.equal( 'foo - A' );
+					expect( view.tooltipView.text ).to.equal( 'foo - A' );
 				} );
 
 				it( 'reacts to changes of #label and #keystroke', () => {
 					view.tooltip = ( l, k ) => `${ l } - ${ k }`;
 					view.label = 'foo';
 					view.keystroke = 'A';
+					view.init();
 
-					expect( view.element.dataset.ckTooltip ).to.equal( 'foo - A' );
+					expect( view.tooltipView.text ).to.equal( 'foo - A' );
 
 					view.label = 'bar';
 					view.keystroke = 'B';
 
-					expect( view.element.dataset.ckTooltip ).to.equal( 'bar - B' );
+					expect( view.tooltipView.text ).to.equal( 'bar - B' );
 				} );
 			} );
 		} );
@@ -218,7 +266,7 @@ describe( 'ButtonView', () => {
 	describe( 'icon', () => {
 		it( 'is not initially set', () => {
 			expect( view.element.childNodes ).to.have.length( 1 );
-			expect( view.iconView ).to.undefined;
+			expect( view.iconView ).to.be.undefined;
 		} );
 
 		it( 'is set when view#icon is defined', () => {

+ 58 - 0
packages/ckeditor5-ui/tests/tooltip/tooltipview.js

@@ -0,0 +1,58 @@
+/**
+ * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import TooltipView from '../../src/tooltip/tooltipview';
+
+describe( 'TooltipView', () => {
+	let view, text;
+
+	beforeEach( () => {
+		view = new TooltipView();
+		text = view.element.firstChild;
+	} );
+
+	describe( 'constructor()', () => {
+		it( 'should create element from template', () => {
+			expect( view.element.tagName ).to.equal( 'SPAN' );
+			expect( view.element.classList.contains( 'ck-tooltip' ) ).to.be.true;
+			expect( view.element.childNodes ).to.have.length( 1 );
+
+			expect( text.tagName ).to.equal( 'SPAN' );
+			expect( text.classList.contains( 'ck-tooltip__text' ) ).to.be.true;
+		} );
+
+		it( 'should set default #position', () => {
+			expect( view.position ).to.equal( 's' );
+		} );
+	} );
+
+	describe( 'DOM bindings', () => {
+		beforeEach( () => {
+			view.text = 'foo';
+		} );
+
+		describe( 'text content', () => {
+			it( 'should react on view#text', () => {
+				expect( text.textContent ).to.equal( 'foo' );
+
+				view.text = 'baz';
+
+				expect( text.textContent ).to.equal( 'baz' );
+			} );
+		} );
+
+		describe( 'class', () => {
+			it( 'should react on view#position', () => {
+				expect( view.element.classList.contains( 'ck-tooltip_n' ) ).to.be.false;
+				expect( view.element.classList.contains( 'ck-tooltip_s' ) ).to.be.true;
+
+				view.position = 'n';
+
+				expect( view.element.classList.contains( 'ck-tooltip_n' ) ).to.be.true;
+				expect( view.element.classList.contains( 'ck-tooltip_s' ) ).to.be.false;
+			} );
+		} );
+	} );
+} );

+ 26 - 56
packages/ckeditor5-ui/theme/components/tooltip.scss

@@ -1,43 +1,12 @@
 // Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
 // For licensing, see LICENSE.md or http://ckeditor.com/license
 
-/**
- * Applies styles to the main part of the tooltip.
- */
-@mixin ck-tooltip__main {
-	&::after {
-		@content;
-	}
-}
-
-/**
- * Applies styles to the arrow part of the tooltip.
- */
-@mixin ck-tooltip__arrow {
-	&::before {
-		@content;
-	}
-}
-
-/**
- * Applies styles to both arrow and main part of the tooltip.
- */
-@mixin ck-tooltip__elements {
-	@include ck-tooltip__main {
-		@content;
-	}
-
-	@include ck-tooltip__arrow {
-		@content;
-	}
-}
-
 /**
  * Enables the tooltip, which is the tooltip is in DOM but
  * not yet displayed.
  */
 @mixin ck-tooltip_enabled {
-	@include ck-tooltip__elements {
+	.ck-tooltip {
 		display: block;
 	}
 }
@@ -46,7 +15,7 @@
  * Disables the tooltip making it disappear from DOM.
  */
 @mixin ck-tooltip_disabled {
-	@include ck-tooltip__elements {
+	.ck-tooltip {
 		display: none;
 	}
 }
@@ -56,37 +25,38 @@
  * Requires `ck-tooltip_enabled` first.
  */
 @mixin ck-tooltip_visible {
-	@include ck-tooltip__elements {
+	.ck-tooltip {
 		visibility: visible;
 		opacity: 1;
 	}
 }
 
-[data-ck-tooltip] {
-	@include ck-tooltip__elements {
-		// Tooltip is hidden by default.
-		visibility: hidden;
-		opacity: 0;
-		display: none;
+.ck-tooltip,
+.ck-tooltip__text::after {
+	position: absolute;
 
-		position: absolute;
-		z-index: ck-z( 'modal' );
+	// Without this, hovering the tooltip could keep it visible.
+	pointer-events: none;
 
-		// Without this, hovering the tooltip could keep it visible.
-		pointer-events: none;
+	// This is to get rid of flickering when transitioning opacity in Chrome.
+	// It's weird but it works.
+	-webkit-backface-visibility: hidden;
+}
 
-		// This is to get rid of flickering when transitioning opacity in Chrome.
-		// It's weird but it works.
-		-webkit-backface-visibility: hidden;
-	}
+.ck-tooltip {
+	// Tooltip is hidden by default.
+	visibility: hidden;
+	opacity: 0;
+	display: none;
+	z-index: ck-z( 'modal' );
+}
 
-	@include ck-tooltip__main {
-		content: attr(data-ck-tooltip);
-	}
+.ck-tooltip__text {
+	display: inline-block;
+}
 
-	@include ck-tooltip__arrow {
-		content: "";
-		width: 0;
-		height: 0;
-	}
+.ck-tooltip__text::after {
+	content: "";
+	width: 0;
+	height: 0;
 }