Merge pull request #280 from ckeditor/t/142

Fix: Button tooltip should not look blurry on ldpi screens. Closes #142. Closes #133.

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

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' ),
+						}
+					]
+				}
+			]
+		} );
+	}
+}

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,51 +76,93 @@ 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', () => {
 				it( 'renders tooltip text out of #label and #keystroke', () => {
 					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', () => {