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

[POC] Shortcut editor (for command mode) #1265

Closed
wants to merge 12 commits into from

Conversation

Carreau
Copy link
Member

@Carreau Carreau commented Mar 25, 2016

Proof of concept.

  1. Ask Webpack to compile es6, because now that we have webpack, why not ?
  2. install react/react-dom.
  3. Learn React, and write a module that allows you to edit keyboard shortcuts.
  4. enable eslint for es6.

Todo:

  • Actually persist the new shortcuts
  • actually persist the shortcut we unbind.
  • design.
  • figure out how to destroy the react components.
  • do not fail silently if :
    • shortcut already attached to another action.
    • Shortcut invalid.
  • decide on JSX.
  • name things correctly.

PIng @rgbkrk and @jdfreder because I have no clue what I am doing.


Personal note, es6 is much less WTF than es5 , but still WTF.
JSX: replace < by ?> and /> by <? and you get PHP.

@rgbkrk
Copy link
Member

rgbkrk commented Mar 25, 2016

Wow, awesome. I look forward to checking this out.

JSX: replace < by ?> and /> by <? and you get PHP.

Now you know Facebook's secret to success and adoption of a library.

@@ -0,0 +1 @@
{ "presets": ["es2015"] }
Copy link
Member

Choose a reason for hiding this comment

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

You'll probably want react in your presets here, especially if you use JSX. I'm fine either way and won't bikeshed on the myriad alternatives.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't use JSX for now. I sticked to doing things manually.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, cool!

@jdfreder
Copy link
Contributor

Dude awesome!

this.props.shortcut?
React.createElement('i', {className: "pull-right fa fa-times", alt: 'remove title'+this.props.shortcut}):
React.createElement('i', {className: "pull-right fa fa-plus", alt: 'add-keyboard-shortcut', onClick:()=>{
that.props.onAddBindings(that.state.shrt, that.props.ckey);
Copy link
Contributor

Choose a reason for hiding this comment

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

sweet arrow func

@jdfreder
Copy link
Contributor

I'm very happy to see with the webpack stuff the transition to es6 is straight forward 👍 👍 👍

@Carreau
Copy link
Member Author

Carreau commented Mar 25, 2016

I'm very happy to see with the webpack stuff the transition to es6 is straight forward

The --watch is a bit slow and re-parse most of what is in components, do you know if there is a way to speed that up ? Or deactivate the optimisations pass on --watch ?

av?that.props.onAddBindings(that.state.shrt, that.props.ckey):undefined;
}
}),
this.props.shortcut? undefined :
Copy link
Member

Choose a reason for hiding this comment

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

When you don't want an element to show, return null.

@Carreau
Copy link
Member Author

Carreau commented Apr 13, 2016

Closing in favor of #1347 that does not have one of the main concern that goes with the PR.

@Carreau Carreau closed this Apr 13, 2016
@rgbkrk
Copy link
Member

rgbkrk commented Apr 13, 2016

The main concern coming from the dark thread about use of React in Jupyter?

@Carreau
Copy link
Member Author

Carreau commented Apr 13, 2016

The main concern coming from the dark thread about use of React in Jupyter?

Yep.

@minrk minrk modified the milestone: no action Apr 15, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants