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
browsh-org · Jul 7, 2018 · 2ea4eb0 · 2ea4eb0
1 parent 6fc594b
commit 2ea4eb0
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,