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

Vim Keybindings #115

Merged
merged 4 commits into from
Sep 13, 2020
Merged

Vim Keybindings #115

merged 4 commits into from
Sep 13, 2020

Conversation

thkim1011
Copy link
Contributor

This is more of a proof of concept (the actual code needs more cleanup work and there are some design questions that need to be addressed here), but I've managed to implement vim keybindings to the frontend. My changes adds a new action menu which allows you to switch from normal mode to vim mode. Vim mode should allow hjkl for navigation and x for deleting in normal mode, and all the original action keys from insert mode. To switch into insert mode use i and to switch back to normal mode use Escpape. If you could look at it and tell me what you think, that'd be great :).

@vercel
Copy link

vercel bot commented Sep 13, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/downforacross/downforacrosscom/f5oa4x90y
✅ Preview: https://downforacrosscom-git-fork-thkim1011-vim-mode.downforacross1.vercel.app


const {onVimNormal, onVimInsert, vimInsert, onPressEnter, onPressPeriod} = this.props;
if (key in actionKeys) {
this.handleAction(actionKeys[key], shiftKey);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return true to prevent tab default behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i noticed this bug but was having trouble figuring out why it was happening. thanks!

x: 'delete',
' ': 'space',
'[': 'backward',
']': 'forward',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

w -> selectNextClue(true), b -> selectNextClue(false) ? :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and maybe dd for delete word, though that's a bit unintuitive when it's vertical i guess

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added w and b (good suggestions!) but haven't added dd yet since it seems like it would take some more work (need to add some queue state to check for the first d. I could take a stab at it in another PR along with some other ideas I had e.g. use :32 to go to clue 32.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh yea, :32 would be sweet

const {vimMode, vimInsert} = this.props;
return (
<ActionMenu
label={vimMode ? `VIM ${vimInsert ? 'Insert' : 'Normal'}` : 'Normal'}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think VIM Normal is a bit confusing at first glance. maybe strings can be

  • Normal
  • VIM
  • VIM (insert)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

Copy link
Member

@stevenhao stevenhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i really like this, thanks for the feature! left some inline suggestions, happy to merge this in once we address those!

@thkim1011
Copy link
Contributor Author

Hi @stevenhao , I made the changes you mentioned.

@stevenhao
Copy link
Member

Amazing :)

@stevenhao stevenhao merged commit e0cec81 into downforacross:master Sep 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants