From 2ea4eb0f8caf7370bc9dec886b13382859e52a05 Mon Sep 17 00:00:00 2001 From: Thomas Buckley-Houston Date: Sat, 7 Jul 2018 18:19:58 +0800 Subject: [PATCH] Better ENTER key UX This came about from using Slack's web client. Adding the input boxes padding to the DOM box coords makes the TTY cursor more closely reflect the actual input box. Also using keyup/down seems more universally applicable than merely kepress --- interfacer/src/browsh/input_box.go | 13 ++++++++++--- interfacer/src/browsh/input_cursor.go | 1 + interfacer/src/browsh/tty.go | 8 ++++++++ webext/src/background/manager.js | 1 + webext/src/dom/commands_mixin.js | 18 ++++++++++++++++-- webext/src/dom/serialise_mixin.js | 6 ++++-- 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/interfacer/src/browsh/input_box.go b/interfacer/src/browsh/input_box.go index de9709d..06dd843 100644 --- a/interfacer/src/browsh/input_box.go +++ b/interfacer/src/browsh/input_box.go @@ -164,7 +164,7 @@ func (i *inputBox) sendInputBoxToBrowser() { sendMessageToWebExtension("/tab_command,/input_box," + string(marshalled)) } -func (i *inputBox) handleEnterKey() { +func (i *inputBox) handleEnterKey(modifier tcell.ModMask) { if urlInputBox.isActive { if isNewEmptyTabActive() { sendMessageToWebExtension("/new_tab," + i.text) @@ -173,9 +173,16 @@ func (i *inputBox) handleEnterKey() { } urlBarFocus(false) } - if i.isMultiLine() { + if i.isMultiLine() && modifier != tcell.ModAlt { i.cursorInsertRune([]rune("\n")[0]) + } else { + i.isActive = false } + if i.isMultiLine() && modifier == tcell.ModAlt { + i.text = "" + i.isActive = true + } + i.updateAllCursors() } func (i *inputBox) selectionOff() { @@ -217,7 +224,7 @@ func handleInputBoxInput(ev *tcell.EventKey) { activeInputBox.cursorBackspace() case tcell.KeyEnter: activeInputBox.removeSelectedText() - activeInputBox.handleEnterKey() + activeInputBox.handleEnterKey(ev.Modifiers()) case tcell.KeyRune: activeInputBox.removeSelectedText() activeInputBox.cursorInsertRune(ev.Rune()) diff --git a/interfacer/src/browsh/input_cursor.go b/interfacer/src/browsh/input_cursor.go index 842d237..12fdf32 100644 --- a/interfacer/src/browsh/input_cursor.go +++ b/interfacer/src/browsh/input_cursor.go @@ -3,6 +3,7 @@ package browsh import "unicode/utf8" func (i *inputBox) renderCursor() { + if !i.isActive { return } if i.isSelection() { i.renderSelectionCursor() } else { diff --git a/interfacer/src/browsh/tty.go b/interfacer/src/browsh/tty.go index 43e11fd..a01da82 100644 --- a/interfacer/src/browsh/tty.go +++ b/interfacer/src/browsh/tty.go @@ -102,6 +102,7 @@ func openHelpTab() { } func forwardKeyPress(ev *tcell.EventKey) { + if isMultiLineEnter(ev) { return } eventMap := map[string]interface{}{ "key": int(ev.Key()), "char": string(ev.Rune()), @@ -111,6 +112,13 @@ func forwardKeyPress(ev *tcell.EventKey) { sendMessageToWebExtension("/stdin," + string(marshalled)) } +// Allow user to use ENTER key without triggering submission on multiline input +// boxes. +func isMultiLineEnter(ev *tcell.EventKey) bool { + if activeInputBox == nil { return false } + return activeInputBox.isMultiLine() && ev.Key() == 13 && ev.Modifiers() != 4 +} + func handleScrolling(ev *tcell.EventKey) { yScrollOriginal := CurrentTab.frame.yScroll _, height := screen.Size() diff --git a/webext/src/background/manager.js b/webext/src/background/manager.js index 29846d3..bbc4789 100644 --- a/webext/src/background/manager.js +++ b/webext/src/background/manager.js @@ -258,6 +258,7 @@ export default class extends utils.mixins(CommonMixin, TTYCommandsMixin) { return true; } + // Listen for HTTP activity so we can notify the user that something is loading in the background _addWebRequestListener() { browser.webRequest.onBeforeRequest.addListener( (e) => { diff --git a/webext/src/dom/commands_mixin.js b/webext/src/dom/commands_mixin.js index 84d232b..d393add 100644 --- a/webext/src/dom/commands_mixin.js +++ b/webext/src/dom/commands_mixin.js @@ -84,6 +84,7 @@ export default (MixinBase) => class extends MixinBase { _handleInputBoxContent(input) { let input_box = document.querySelectorAll(`[data-browsh-id="${input.id}"]`)[0]; if (input_box) { + input_box.focus(); if (input_box.getAttribute('role') == 'textbox') { input_box.innerHTML = input.text; } else { @@ -139,10 +140,23 @@ export default (MixinBase) => class extends MixinBase { _triggerKeyPress(key) { let el = document.activeElement; - el.dispatchEvent(new KeyboardEvent('keypress', { + const key_object = { 'key': key.char, 'keyCode': key.key - })); + }; + let event_press = new KeyboardEvent('keypress', key_object); + let event_down = new KeyboardEvent('keydown', key_object); + let event_up = new KeyboardEvent('keyup', key_object); + // Generally sending down/up serves more use cases. But default input forms + // don't listen for down/up to make the form submit. So this makes the assumption + // that it's okay to send ENTER twice to an input box without any serious side + // effects. + if (key.key === 13 && el.tagName === 'INPUT') { + el.dispatchEvent(event_press); + } else { + el.dispatchEvent(event_down); + el.dispatchEvent(event_up); + } } _mouseAction(type, x, y) { diff --git a/webext/src/dom/serialise_mixin.js b/webext/src/dom/serialise_mixin.js index d3e0710..14b4c0d 100644 --- a/webext/src/dom/serialise_mixin.js +++ b/webext/src/dom/serialise_mixin.js @@ -134,11 +134,13 @@ export default (MixinBase) => class extends MixinBase { } styles = window.getComputedStyle(i); font_rgb = styles['color'].replace(/[^\d,]/g, '').split(',').map((i) => parseInt(i)); + const padding_top = parseInt(styles['padding-top'].replace('px', '')); + const padding_left = parseInt(styles['padding-left'].replace('px', '')); if (this._isUnwantedInboxBox(i, styles)) { return } parsed_input_boxes[i.getAttribute('data-browsh-id')] = { id: i.getAttribute('data-browsh-id'), - x: utils.snap(dom_rect.left * this.dimensions.scale_factor.width), - y: utils.snap(dom_rect.top * this.dimensions.scale_factor.height), + x: utils.snap((dom_rect.left + padding_left) * this.dimensions.scale_factor.width), + y: utils.snap((dom_rect.top + padding_top) * this.dimensions.scale_factor.height), width: width, height: height, tag_name: i.nodeName,