[Libreoffice-commits] online.git: cypress_test/integration_tests loleaflet/src

Ashod Nakashian (via logerrit) logerrit at kemper.freedesktop.org
Wed Mar 18 06:30:27 UTC 2020


 cypress_test/integration_tests/common/helper.js                     |   20 +++
 cypress_test/integration_tests/common/impress.js                    |    2 
 cypress_test/integration_tests/mobile/writer/bottom_toolbar_spec.js |   60 ++++++++++
 loleaflet/src/control/Control.Toolbar.js                            |    6 -
 loleaflet/src/layer/marker/TextInput.js                             |   15 ++
 loleaflet/src/map/Map.js                                            |    4 
 6 files changed, 105 insertions(+), 2 deletions(-)

New commits:
commit 3bafa7d4abdac77ee0e1d840096ce31536ec5acc
Author:     Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Tue Mar 17 12:02:59 2020 -0400
Commit:     Ashod Nakashian <ashnakash at gmail.com>
CommitDate: Wed Mar 18 07:30:08 2020 +0100

    leaflet: maintain the keyboard state after toolbar actions
    
    Previously we supressed the keyboard after toolbar
    actions, but that is problematic because then the
    user is unable to delete/replace selected text,
    because the keyboard is hidden and there is no way
    of showing keyboard without changing the selection.
    
    Now we maintain the keyboard state, which is likely
    visible, since a selection shows the keyboard.
    
    This might not be ideal, because the user might
    hide the keyboard on the device and we will restore
    it after invoking a toolbar action, but at least
    it's more usable now. Unfortunately, there is no
    API to track the keyboard visibility.
    
    New Cypress tests added to validate the above.
    
    The tests depend on checking the last keyboard
    visibility state in Map, because there is no
    reliable (or any?) way to know whether the keyboard
    is visible or not. There are many cases where we
    actually hide the keyboard, while having the input
    focus on the textarea element, so that is no indication
    that the keyboard is visible. We do this for usability
    purposes. For example, during zooming, when selecting
    cells and graphics/shapes, etc.
    
    The purpose of the cell is to validate that we restored
    the keyboard visibility or we changed it, depending
    on which is expected after each operation.
    
    Change-Id: If0f2e96909ff20753043734789397d190cedb502
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90663
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/cypress_test/integration_tests/common/helper.js b/cypress_test/integration_tests/common/helper.js
index 01b3bbff5..849b71cab 100644
--- a/cypress_test/integration_tests/common/helper.js
+++ b/cypress_test/integration_tests/common/helper.js
@@ -57,6 +57,22 @@ function enableEditingMobile() {
 		});
 }
 
+// Assert that NO keyboard input is accepted (i.e. keyboard should be HIDDEN).
+function assertNoKeyboardInput() {
+	cy.window().then(win => {
+		var acceptInput = win.map._textInput.shouldAcceptInput();
+		expect(acceptInput, 'Should accept input').to.equal(false);
+	});
+}
+
+// Assert that keyboard input is accepted (i.e. keyboard should be VISIBLE).
+function assertHaveKeyboardInput() {
+	cy.window().then(win => {
+		var acceptInput = win.map._textInput.shouldAcceptInput();
+		expect(acceptInput, 'Should accept input').to.equal(true);
+	});
+}
+
 // Assert that we have cursor and focus.
 function assertCursorAndFocus() {
 	cy.log('Verifying Cursor and Focus.');
@@ -67,6 +83,8 @@ function assertCursorAndFocus() {
 	cy.get('.leaflet-cursor-container')
 		.should('exist');
 
+	assertHaveKeyboardInput();
+
 	cy.log('Cursor and Focus verified.');
 }
 
@@ -195,6 +213,8 @@ function longPressOnDocument(posX, posY) {
 module.exports.loadTestDoc = loadTestDoc;
 module.exports.enableEditingMobile = enableEditingMobile;
 module.exports.assertCursorAndFocus = assertCursorAndFocus;
+module.exports.assertNoKeyboardInput = assertNoKeyboardInput;
+module.exports.assertHaveKeyboardInput = assertHaveKeyboardInput;
 module.exports.selectAllText = selectAllText;
 module.exports.clearAllText = clearAllText;
 module.exports.getTextForClipboard = getTextForClipboard;
diff --git a/cypress_test/integration_tests/common/impress.js b/cypress_test/integration_tests/common/impress.js
index db06ea9ef..d6463fafc 100644
--- a/cypress_test/integration_tests/common/impress.js
+++ b/cypress_test/integration_tests/common/impress.js
@@ -12,6 +12,8 @@ function assertNotInTextEditMode() {
 	cy.get('.leaflet-cursor-container')
 		.should('not.exist');
 
+	helper.assertNoKeyboardInput();
+
 	cy.log('NO Text-Edit context verified.');
 }
 
diff --git a/cypress_test/integration_tests/mobile/writer/bottom_toolbar_spec.js b/cypress_test/integration_tests/mobile/writer/bottom_toolbar_spec.js
index 7bf407442..e9dfd0ebf 100644
--- a/cypress_test/integration_tests/mobile/writer/bottom_toolbar_spec.js
+++ b/cypress_test/integration_tests/mobile/writer/bottom_toolbar_spec.js
@@ -34,6 +34,26 @@ describe('Pushing bottom toolbar items.', function() {
 			.should('exist');
 	});
 
+	it('Apply bold, check keyboard.', function() {
+		cy.get('#tb_editbar_item_bold div table')
+			.should('not.have.class', 'checked');
+
+		cy.window().then(win => {
+			win.lastInputState = win.map._textInput.shouldAcceptInput();
+		});
+
+		cy.get('#tb_editbar_item_bold')
+			.click();
+
+		cy.get('#tb_editbar_item_bold div table')
+			.should('have.class', 'checked');
+
+		cy.window().then(win => {
+			var acceptInput = win.map._textInput.shouldAcceptInput();
+			expect(acceptInput, 'Should accept input').to.equal(win.lastInputState);
+		});
+	});
+
 	it('Apply italic.', function() {
 		cy.get('#tb_editbar_item_italic div table')
 			.should('not.have.class', 'checked');
@@ -50,6 +70,26 @@ describe('Pushing bottom toolbar items.', function() {
 			.should('exist');
 	});
 
+	it('Apply italic, check keyboard.', function() {
+		cy.get('#tb_editbar_item_italic div table')
+			.should('not.have.class', 'checked');
+
+		cy.window().then(win => {
+			win.lastInputState = win.map._textInput.shouldAcceptInput();
+		});
+
+		cy.get('#tb_editbar_item_italic')
+			.click();
+
+		cy.get('#tb_editbar_item_italic div table')
+			.should('have.class', 'checked');
+
+		cy.window().then(win => {
+			var acceptInput = win.map._textInput.shouldAcceptInput();
+			expect(acceptInput, 'Should accept input').to.equal(win.lastInputState);
+		});
+	});
+
 	it('Apply underline.', function() {
 		cy.get('#tb_editbar_item_underline div table')
 			.should('not.have.class', 'checked');
@@ -66,6 +106,26 @@ describe('Pushing bottom toolbar items.', function() {
 			.should('exist');
 	});
 
+	it('Apply underline, check keyboard.', function() {
+		cy.get('#tb_editbar_item_underline div table')
+			.should('not.have.class', 'checked');
+
+		cy.window().then(win => {
+			win.lastInputState = win.map._textInput.shouldAcceptInput();
+		});
+
+		cy.get('#tb_editbar_item_underline')
+			.click();
+
+		cy.get('#tb_editbar_item_underline div table')
+			.should('have.class', 'checked');
+
+		cy.window().then(win => {
+			var acceptInput = win.map._textInput.shouldAcceptInput();
+			expect(acceptInput, 'Should accept input').to.equal(win.lastInputState);
+		});
+	});
+
 	it('Apply strikeout.', function() {
 		cy.get('#tb_editbar_item_strikeout div table')
 			.should('not.have.class', 'checked');
diff --git a/loleaflet/src/control/Control.Toolbar.js b/loleaflet/src/control/Control.Toolbar.js
index 79d392563..ea80836a4 100644
--- a/loleaflet/src/control/Control.Toolbar.js
+++ b/loleaflet/src/control/Control.Toolbar.js
@@ -150,15 +150,17 @@ function onClick(e, id, item, subItem) {
 	else {
 		throw new Error('unknown id: ' + id);
 	}
-	var docLayer = map._docLayer;
+
 	// In the iOS app we don't want clicking on the toolbar to pop up the keyboard.
 	if (!window.ThisIsTheiOSApp && id !== 'zoomin' && id !== 'zoomout' && id !== 'mobile_wizard' && id !== 'insertion_mobile_wizard') {
-		map.focus();
+		map.focus(map.shouldAcceptInput()); // Maintain same keyboard state.
 	}
+
 	if (item.disabled) {
 		return;
 	}
 
+	var docLayer = map._docLayer;
 	if (item.postmessage && item.type === 'button') {
 		map.fire('postMessage', {msgId: 'Clicked_Button', args: {Id: item.id} });
 	}
diff --git a/loleaflet/src/layer/marker/TextInput.js b/loleaflet/src/layer/marker/TextInput.js
index 98138e761..0495e077c 100644
--- a/loleaflet/src/layer/marker/TextInput.js
+++ b/loleaflet/src/layer/marker/TextInput.js
@@ -27,6 +27,10 @@ L.TextInput = L.Layer.extend({
 		// Clearing the area can generate input events
 		this._ignoreInputCount = 0;
 
+		// If the last focus intended to accept user input.
+		// Signifies whether the keyboard is meant to be visible.
+		this._acceptInput = false;
+
 		// Content
 		this._lastContent = []; // unicode characters
 		this._hasWorkingSelectionStart = undefined; // does it work ?
@@ -158,6 +162,7 @@ L.TextInput = L.Layer.extend({
 		// container in order for the user to input text (and on-screen keyboards
 		// to pop-up), unless the document is read only.
 		if (this._map._permission !== 'edit') {
+			this._acceptInput = false;
 			return;
 		}
 
@@ -169,15 +174,25 @@ L.TextInput = L.Layer.extend({
 		this._textArea.focus();
 
 		if ((window.ThisIsAMobileApp || window.mode.isMobile()) && acceptInput !== true) {
+			this._acceptInput = false;
 			this._textArea.blur();
 			this._textArea.removeAttribute('readonly');
+		} else {
+			this._acceptInput = true;
 		}
 	},
 
 	blur: function() {
+		this._acceptInput = false;
 		this._textArea.blur();
 	},
 
+	// Returns true if the last focus was to accept input.
+	// Used to restore the keyboard.
+	shouldAcceptInput: function() {
+		return this._acceptInput;
+	},
+
 	// Marks the content of the textarea/contenteditable as selected,
 	// for system clipboard interaction.
 	select: function select() {
diff --git a/loleaflet/src/map/Map.js b/loleaflet/src/map/Map.js
index 4826cee18..600a078d3 100644
--- a/loleaflet/src/map/Map.js
+++ b/loleaflet/src/map/Map.js
@@ -946,6 +946,10 @@ L.Map = L.Evented.extend({
 		return document.activeElement === this._textInput.activeElement();
 	},
 
+	shouldAcceptInput: function() {
+		return this._textInput.shouldAcceptInput();
+	},
+
 	setHelpTarget: function(page) {
 		this._helpTarget = page;
 	},


More information about the Libreoffice-commits mailing list