hypergravity icon indicating copy to clipboard operation
hypergravity copied to clipboard

Not working on hyper 2.0

Open spoike opened this issue 6 years ago • 9 comments

Doesn't seem to work on hyper 2.0 (neither stable nor canary). Hitting the shortcut keys Ctrl/Cmd+G does nothing. No errors on console.

I can see that the plugin is loaded in Dev Tools:

Plugin hypergravity (1.0.5) loaded.

Running hyper on on MacOS High Sierra.

spoike avatar Apr 18 '18 07:04 spoike

Looks like the keyboard shortcut API changed. I'll try to find some docs on the new API and fit the plugin to work with the new and the 1.x API. Contributions welcome! ;)

krzema12 avatar Apr 18 '18 09:04 krzema12

Here's a clue: https://github.com/xtermjs/xterm.js/blob/ceffb354453c21722a555914f8d068fc4e7f244b/src/Terminal.ts#L1328-L1339 Hyper now uses XTerm that exposes the above API.

krzema12 avatar Apr 18 '18 09:04 krzema12

Did the following patch to use decorateKeymaps instead of manually registering the key mapping.

diff --git a/src/index.js b/src/index.js
index 870c870..139151f 100644
--- a/src/index.js
+++ b/src/index.js
@@ -3,6 +3,12 @@ import GravityAnimationModel from './GravityAnimationModel';
 
 import DOMUtils from './DOMUtils';
 
+exports.decorateKeymaps = (keymaps) => {
+  return Object.assign({}, keymaps, {
+    'gravity:toggle': 'ctrl+g'
+  });
+}
+
 exports.decorateTerm = (Term, { React, notify }) => {
   return class extends React.Component {
     constructor (props, context) {
@@ -13,14 +19,31 @@ exports.decorateTerm = (Term, { React, notify }) => {
       this._selectDOMElementsToAnimate = this._selectDOMElementsToAnimate.bind(this);
       this._elements = [];
       this._elmentSelectionStrategy = new TextNodesElementSelectionStrategy();
+      this._terms = null;
+      this._onDecorated = this._onDecorated.bind(this);
     }
 
     render () {
       return React.createElement(Term, Object.assign({}, this.props, {
-        onTerminal: this._onTerminal
+        onTerminal: this._onTerminal,
+        onDecorated: this._onDecorated
       }));
     }
 
+    _onDecorated (terms) {
+      if (this.props.onDecorated) { this.props.onDecorated(terms); };
+
+      this._terms = terms;
+      console.log('registering gravity');
+      terms.registerCommands({
+        'gravity:toggle': (e) => {
+          console.log('toggling gravity mode');
+          this._toggleGravityMode();
+          e.preventDefault();
+        }
+      })
+    }
+
     _drawFrame() {
       if (!this._isGravityEnabled) {
         return;
@@ -70,26 +93,6 @@ exports.decorateTerm = (Term, { React, notify }) => {
       }
 
       this._rootDiv = term.div_;
-
-      this._addKeyboardShortcutHandler(term);
-    }
-
-    _addKeyboardShortcutHandler(term) {
-      const activatingKeyShortcutHandler = [
-        "keydown",
-        function(e) {
-          if ((  (window.process.platform === 'darwin' &&  e.metaKey && !e.ctrlKey)
-              || (window.process.platform !== 'darwin' && !e.metaKey &&  e.ctrlKey))
-              && !e.shiftKey && e.keyCode === 'G'.charCodeAt(0)) {
-            this._toggleGravityMode();
-            console.log('Gravity mode ' + (this._isGravityEnabled ? 'enabled' : 'disabled'));
-          }
-        }.bind(this)
-      ];
-
-      term.uninstallKeyboard();
-      term.keyboard.handlers_ = [...term.keyboard.handlers_, activatingKeyShortcutHandler];
-      term.installKeyboard();
     }
 
     _toggleGravityMode() {

Unfortunately the plugin appears to crash other plugins because a DOM reference is missing after this plugin is loaded. Not really sure whyeactly. But have a go at the patch if you can figure that out.

spoike avatar Apr 18 '18 09:04 spoike

Can you paste any logs that show other plugins crash? Does pressing Ctrl+G toggle gravity after your change?

krzema12 avatar Apr 18 '18 09:04 krzema12

hyperpower seems to crash with an Uncaught TypeError when it is enabled together with the patched hypergravity.

[Notification] Plugin error: Error occurred. Check Developer Tools for details
/Users/spoike/.hyper_plugins/node_modules/hyperpower/index.js:207 Uncaught TypeError: Cannot read property 'getBoundingClientRect' of null
    at Object._onCursorMove (/Users/spoike/.hyper_plugins/node_modules/hyperpower/index.js:207)
    at Terminal.e.onCursorMove.term.on (bundle.js:1)
    at Terminal.EventEmitter.emit (bundle.js:1)
    at Parser.parse (bundle.js:1)
    at Terminal._innerWrite (bundle.js:1)
    at bundle.js:1

It appears to also make the terminal unresponsive to key events. Probably because the crashed plugin was wrapping the key events.


Disabling hyperpower the other plugins I'm currently using (hyperterm-visor, hyper-spotify) doesn't seem to throw errors but gives the notification that something has happened:

[Notification] Plugin error: Error occurred. Check Developer Tools for details

At that point using the shortcut (ctrl+g) does nothing (the handler in terms.registerCommands appears not to run).

spoike avatar Apr 18 '18 10:04 spoike

About hyperpower crashing: are you sure you're running the newest version of it? Something was changed 2 days ago around compatibility with Hyper 2.0: https://github.com/zeit/hyperpower/commit/c9ed3e412414d7286ae8c09908e73053001b8aff

Also, please try with some other shortcut that Ctrl+G. There may be a collision with another plugin registering to this event.

I'll try to reproduce it on weekend.

krzema12 avatar Apr 18 '18 13:04 krzema12

It appears I misread the docs. There are two similarly named decorator functions decorateTerm and decorateTerms. The registerCommands function is only available on the second decorator function, which makes sense since you only want to register the keymap on a level above individual terminal.

I can now activate the gravity mode with the shortcut. Unfortunately, since hyper 2.0 is now using a canvas based terminal renderer the plugin now throws errors of nodes not being nodes. Error:

Uncaught TypeError: Cannot read property 'nodeType' of undefined
    at TopLevelNodesElementSelectionStrategy._isNonEmptyTextNode (/Users/spoike/.hyper_plugins/local/hypergravity/index.js:352)
    at TopLevelNodesElementSelectionStrategy.shouldAnimateElement (/Users/spoike/.hyper_plugins/local/hypergravity/index.js:348)
    at Object._selectDOMElementsToAnimate (/Users/spoike/.hyper_plugins/local/hypergravity/index.js:256)
    at Object._enableGravityMode (/Users/spoike/.hyper_plugins/local/hypergravity/index.js:231)
    at Object._toggleGravityMode (/Users/spoike/.hyper_plugins/local/hypergravity/index.js:226)
    at Object._toggleGravityMode (/Users/spoike/.hyper_plugins/local/hypergravity/index.js:141)
    at gravity:toggle (/Users/spoike/.hyper_plugins/local/hypergravity/index.js:125)
    at Object.effect (bundle.js:1)
    at bundle.js:1
    at bundle.js:1

Here is the patch with the functional keybinding tho':

diff --git a/src/index.js b/src/index.js
index 870c870..240d4c6 100644
--- a/src/index.js
+++ b/src/index.js
@@ -3,6 +3,55 @@ import GravityAnimationModel from './GravityAnimationModel';
 
 import DOMUtils from './DOMUtils';
 
+exports.decorateKeymaps = (keymaps) => {
+  return Object.assign({}, keymaps, {
+    'gravity:toggle': 'ctrl+g'
+  });
+}
+
+exports.decorateTerms = (Terms, {React}) => {
+  return class extends React.Component {
+    constructor (props, context) {
+      super(props, context);
+
+      this._onDecorated = this._onDecorated.bind(this);
+      this._terms = null;
+    }
+
+    render () {
+      return React.createElement(Terms, Object.assign({}, this.props, {
+        onDecorated: this._onDecorated
+      }));
+    }
+
+    _onDecorated (terms) {
+      if (this.props.onDecorated) { this.props.onDecorated(terms); };
+
+      if (terms) {
+        terms.registerCommands({
+          'gravity:toggle': (e) => {
+            this._toggleGravityMode();
+            e.preventDefault();
+          }
+        });
+      }
+
+      this._terms = terms;
+    }
+
+    _toggleGravityMode () {
+      if (!this._terms.getActiveTerm) {
+        return;
+      }
+      const term = this._terms.getActiveTerm();
+      console.log('term', term);
+      if (term && term.props && term.props.toggleGravityMode) {
+        term.props.toggleGravityMode();
+      }
+    }
+  };
+};
+
 exports.decorateTerm = (Term, { React, notify }) => {
   return class extends React.Component {
     constructor (props, context) {
@@ -10,6 +59,9 @@ exports.decorateTerm = (Term, { React, notify }) => {
 
       this._onTerminal = this._onTerminal.bind(this);
       this._drawFrame = this._drawFrame.bind(this);
+      this._toggleGravityMode = this._toggleGravityMode.bind(this);
+      this._enableGravityMode = this._enableGravityMode.bind(this);
+      this._disableGravityMode = this._disableGravityMode.bind(this);
       this._selectDOMElementsToAnimate = this._selectDOMElementsToAnimate.bind(this);
       this._elements = [];
       this._elmentSelectionStrategy = new TextNodesElementSelectionStrategy();
@@ -17,7 +69,8 @@ exports.decorateTerm = (Term, { React, notify }) => {
 
     render () {
       return React.createElement(Term, Object.assign({}, this.props, {
-        onTerminal: this._onTerminal
+        onTerminal: this._onTerminal,
+        toggleGravityMode: this._toggleGravityMode
       }));
     }
 
@@ -70,32 +123,14 @@ exports.decorateTerm = (Term, { React, notify }) => {
       }
 
       this._rootDiv = term.div_;
-
-      this._addKeyboardShortcutHandler(term);
-    }
-
-    _addKeyboardShortcutHandler(term) {
-      const activatingKeyShortcutHandler = [
-        "keydown",
-        function(e) {
-          if ((  (window.process.platform === 'darwin' &&  e.metaKey && !e.ctrlKey)
-              || (window.process.platform !== 'darwin' && !e.metaKey &&  e.ctrlKey))
-              && !e.shiftKey && e.keyCode === 'G'.charCodeAt(0)) {
-            this._toggleGravityMode();
-            console.log('Gravity mode ' + (this._isGravityEnabled ? 'enabled' : 'disabled'));
-          }
-        }.bind(this)
-      ];
-
-      term.uninstallKeyboard();
-      term.keyboard.handlers_ = [...term.keyboard.handlers_, activatingKeyShortcutHandler];
-      term.installKeyboard();
     }
 
     _toggleGravityMode() {
       if (this._isGravityEnabled) {
+        console.log('enabled');
         this._disableGravityMode();
       } else {
+        console.log('disabled');
         this._enableGravityMode();
       }
     }

spoike avatar Apr 19 '18 07:04 spoike

Created PR #21 with the patch (sans the console.log) so you can tinker with it. ;-)

spoike avatar Apr 19 '18 07:04 spoike

@spoike , thanks a lot for your time. I had several minutes to take a look at it and:

  • I tested your patch on Hyper 2.0.0 and it works with Ctrl+G on a Mac now, not Command+G, which is generally ok. The bigger problem is that...
  • for real, Hyper now uses canvas to display the content of the terminal

It means that either hypergravity is dead, or it needs a major rewrite to support Hyper 2.0. I'm not even sure if it's still possible to achieve the same effect. In Hyper 1.x, it was just about direct access to the DOM, and now, probably some API had to be used to at least get the elements. Definitely not a trivial thing, requiring to dive deeper into Hyper, maybe hacking it a bit.

The good news is that there's e.g. https://github.com/zeit/hyperpower that relies on being able to modify the elements' position for the shaking effect. I tested if it works in Hyper 2.x. Let's wait for them to apply some action, maybe it will inspire us how to fix hypergravity.

If anyone has ideas how to approach it or perhaps a proof of concept, I'd appreciate them!

krzema12 avatar Apr 26 '18 05:04 krzema12