Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for keypress events #138

Closed
sindresorhus opened this issue Mar 7, 2019 · 21 comments
Closed

Add support for keypress events #138

sindresorhus opened this issue Mar 7, 2019 · 21 comments

Comments

@sindresorhus
Copy link
Collaborator

To be able to have the component rerendered on keypresses and get the keys.

Ink would need to do readline.emitKeypressEvents(stdin); in https://github.com/vadimdemedes/ink/blob/master/src/components/App.js#L61 And it requires raw-mode.

After enabling the above, Ink could do something like this internally:

process.stdin.on('keypress', (str, key) => {
    if (key.ctrl && key.name === 'f') {
        console.log('foo')
    }
})

My initial idea was to expose a useKeypress() React hook. @vadimdemedes suggested we could just expose the keypresses by default if raw-mode is activated, as the output is being diffed anyway, so the cost is negligible.

@jedahan
Copy link

jedahan commented Mar 7, 2019

what im doing currently as an example:

const useKeyHandler = keyHandler => {
  const { stdin, setRawMode } = useContext(StdinContext)

  useEffect(() => {
    setRawMode(true)
    stdin.on('data', keyHandler)
    return () => {
      stdin.off('data', keyHandler)
      setRawMode(false)
    }
  }, [stdin, setRawMode])
}
...snip
 // handle keypresses
  useKeyHandler(data => {
    if (data === 's') setSyncing(syncing => !syncing)
    if (data === 'p') setCreature(ascii())
    if (data === 'q') process.exit(0)
  })

from https://github.com/jedahan/creature/blob/ink/src/App.jsx

@jedahan
Copy link

jedahan commented Mar 7, 2019

The suggestion to just expose the handler seems nicer for those coming from nodeland - but i wonder if you have different keys in different contexts if the reactish way helps with multi-screen apps...

@SimenB
Copy link
Contributor

SimenB commented Mar 7, 2019

Not sure if needed, but I have a use case for this. I've been looking at rewriting https://github.com/jest-community/jest-watch-typeahead to use ink.

The thing I want to achieve is this:
demo

So an text input, and below it a list over hits. If the user hits the down arrow, then I want to select from the list.

It might be enough for https://github.com/vadimdemedes/ink-select-input to just have a mode for "none selected" instead of starting with the first in the list as "selected"? But I think I can also listen for keypresses myself and only make the select I render focus={true} if arrow down is pressed. I also need to know when enter is used for the text input

@vadimdemedes
Copy link
Owner

vadimdemedes commented Mar 8, 2019

Ink 2.0.4 now enables keypress events by default now, when raw mode is turned on.

As for React hook for keypress events, I like @jedahan's solution (only with keypress events, not data), simple and clean. @sindresorhus what do you think?

@vadimdemedes
Copy link
Owner

It might be enough for vadimdemedes/ink-select-input to just have a mode for "none selected" instead of starting with the first in the list as "selected"? But I think I can also listen for keypresses myself and only make the select I render focus={true} if arrow down is pressed. I also need to know when enter is used for the text input

@SimenB I think the focus solution is better. If <SelectInput> doesn't show an arrow by default, it will not be obvious that this is a selectable list, it will look like multiple labels on each line :)

@sindresorhus
Copy link
Collaborator Author

As for React hook for keypress events, I like @jedahan's solution (only with keypress events, not data), simple and clean. @sindresorhus what do you think?

I agree. @jedahan Would you be interested in doing a PR?

@SimenB
Copy link
Contributor

SimenB commented Mar 8, 2019

@SimenB I think the focus solution is better. If doesn't show an arrow by default, it will not be obvious that this is a selectable list, it will look like multiple labels on each line :)

I didn't mean for it to be the default behavior, just a prop 🙂

However, setting focus={false} doesn't seem to change anything visually - it just means it doesn't respond to keyevents, it still highlights the first choice. However, I can open up an issue in that repo - this is not the place for it.

EDIT: vadimdemedes/ink-select-input#13


Back to keypresses - I tried this code in 2.0.4:

// copy of @jedahan's hook with `keypress`
function useKeyHandler(keyHandler) {
  const { stdin, setRawMode } = React.useContext(StdinContext);

  React.useEffect(() => {
    setRawMode(true);
    stdin.on('keypress', keyHandler);
    return () => {
      stdin.off('keypress', keyHandler);
      setRawMode(false);
    };
  }, [stdin, setRawMode]);
}

function PromptThing() {
  useKeyHandler(() => {});

  const items = [
    { label: 'hah', value: 'thing number 1' },
    { label: 'hah2', value: 'thing number 2' },
  ];

  return (
    <Box>
      <SelectInput items={items} limit={10} />
    </Box>
  );
}

render(<PromptThing />);

It seems to completely mess up SelectInput. If you run that and press down, it seems you scroll way past the list (wrapping doesn't work?), and have to click up a bunch of times to get to the list again. Pressing up is even more interesting - it swaps the position of the 2 entries and adds newlines above them

@vadimdemedes
Copy link
Owner

Interesting, unused useKeyHandler affects <SelectInput>. I will look into it after work today.

@jedahan jedahan mentioned this issue Mar 8, 2019
4 tasks
@vadimdemedes
Copy link
Owner

Ok, it's the limit that's screwing things up, not the keypress hook. <SelectInput> doesn't properly handle cases when there are less items than limit. Fix coming up!

@vadimdemedes
Copy link
Owner

While fixing this, discovered another interesting issue with reordering children with keys inside a container. Pushed a fix here with a detailed description inside commit: a022a38.

@vadimdemedes
Copy link
Owner

@SimenB <SelectInput> issue with output is fixed in 3.0.2 and I would also recommend to update Ink to 2.0.5, it contains an important reconciler fix.

@SimenB
Copy link
Contributor

SimenB commented Mar 9, 2019

Nice, thanks @vadimdemedes! Works great now 🙂

@xrd
Copy link

xrd commented Mar 13, 2019

Thanks for your suggestions @jedahan.

I'm taking your code and am having trouble with a simple example. Is this just a misunderstanding of hooks, or is there a bug in useKeyHandler?

When I run this code (using ink 2.0.5), I can never get it to go past index===1, and pressing the "k" key always sets index to -1.

import React, { useState, useEffect, useContext } from "react";
import { render, StdinContext, Color, Box } from "ink";

const useKeyHandler = keyHandler => {
  const { stdin, setRawMode } = useContext(StdinContext);

  useEffect(() => {
    setRawMode(true);
    stdin.on("data", keyHandler);
    return () => {
      stdin.off("data", keyHandler);
      setRawMode(false);
    };
  }, [stdin, setRawMode]);
};

// since we have one handler for the whole app, our state lives up here instead of in the functions, maybe there is another way?
const App = () => {
  const [index, setIndex] = useState(0);

  useKeyHandler(data => {
    if (data === "j") setIndex(index + 1);
    if (data === "k") setIndex(index - 1);
  });

  return <IssuesWrapper index={index} />;
};

const IssuesWrapper = ({ index }) => {
  return (
    <Box>
      <Box>
        <Issues index={index} />
      </Box>

      <Box>
        <>Press 'j' to move up</>
        <>Press 'k' to move down</>
      </Box>
    </Box>
  );
};

const Issues = ({ index }) => {
  const [issues, setIssues] = useState([
    { id: "abc", title: "Something" },
    { id: "def", title: "Nothing" },
    { id: "213123", title: "Perspective" },
    { id: "34242", title: "Getting to nothing" }
  ]);

  return (
    <Box paddingTop={2} paddingBottom={2} flexDirection="column">
      <Box paddingBottom={2}>Current index is: {index}</Box>
      <Box flexDirection="column">
        {issues &&
          issues.map((issue, i) => {
            const { id, title } = issue;
            return (
              <Box key={i}>
                <Color bold={i === index}>
                  {id} :: {title}
                </Color>
              </Box>
            );
          })}
      </Box>
    </Box>
  );
};

render(<App />);

@vadimdemedes
Copy link
Owner

@xrd The issue here is how JavaScript works :) index will always reference initial value, which is zero:

const App = () => {
	const [index, setIndex] = useState(0);

	useKeyHandler(data => {
		setIndex(index + 1); // 0 + 1 = 1
	});
};

Try passing a function to setIndex instead, which will receive previous state as an argument:

setIndex(prevIndex => prevIndex + 1);
setIndex(prevIndex => prevIndex - 1);

@xrd
Copy link

xrd commented Mar 14, 2019

@vadimdemedes Thank you!

@KyleAMathews
Copy link

useKeyHandler worked perfect for my app — perhaps this could get published as a package? ink-use-key-handler?

@sindresorhus
Copy link
Collaborator Author

I would like useKeyHandler to be built-in. It's a very common need.

@visigoth
Copy link

i started hacking around with ink and thought that react-hotkeys would be a great way to handle navigation and interaction around CLI applications. finding that ink doesn't have event dispatch, i thought it might be nice if ink utilized jsdom because it will have the kind of event dispatch react applications are already built with and react developers are familiar with. also, a DOM implementation like jsdom has focus management and will dispatch events like blur and focus.

so, i sought out ways to integrate ink with jsdom. i'm pretty close in #209. it's not done yet - ink-box does not render borders correctly at the moment. but, i think allowing for the use of a full featured DOM will open up ink to being useful for a lot more react code that is out there.

@visigoth
Copy link

ah, using the ink reconciler worked! ink-box now renders correctly and i can use react-hotkeys to move around with focus events as well.

@visigoth
Copy link

hm scratch that - see my comment in #209 - turns out react-dom is not happy with unstable__transformChildren and this makes it difficult to render stuff properly. my previous comment was incorrect due to a mismatch between the ink reconciler and react-dom.

@vadimdemedes
Copy link
Owner

Released 2.4.0, which adds useInput hook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants