Closed WNewhook closed 7 years ago
@WNewhook Just did some testing on this and it looks good to me! (Note: It took me a sec to figure out how creating the 'var decl' node would happen on mac/windows in Chrome because of cmd/ctrl+shift+n opening a new incognito window, but if Windows users stick to the meta/'windows' key(I assume) and Mac users stick to 'control' it should work fine. 👍 )
@jillhancock I didn't realize ctrl-shift-n was a chrome shortcut. I don't think there's any way to trigger the meta key on a windows machine - the windows button doesn't seem to do it anyway. Would alt+n work instead? Or maybe something like ctrl-shift-v (for 'variable')?
@WNewhook Oh okay, I just assumed 'windows' key did it. Alt-n on windows-chrome is a shortcut for 'Move focus to a notification', but ctrl/cmd+shift+v isn't used for anything on either. Seems like a good bet. (Alt+v isn't used anywhere either!)
@jillhancock I'll go with ctrl/cmd+shift+v for now.
@christopher-rodgers @jillhancock Firefox apparently reports :/;, +/=, and _/- with different keycodes than other browsers. I changed it so it checks for both Firefox's and Chrome's keycodes now. Can you try it out again?
Everything seemed to work fine for me on both Chrome and Firefox (on Windows) as well as Firefox on Linux.
I do have one suggestion though: should it automatically enter a literal when pressing a number key? Look at this picture for example:
The first node (x = 66) is what happens when you press { : } { ENTER } { RIGHT ARROW } { x } { ENTER } { RIGHT ARROW } { 6 } { 6 } { ENTER } which appears to set the variable x to the value of the variable 66. This acts differently than I would expect, as I expect the case with the second node to happen, which currently requires someone to first press { " } to enter a literal.
I think pressing { " } to enter literals makes sense (since most languages require it for string literals), but when I enter a number I expect it to be a literal, as it is standard behaviour in all other languages I have worked with.
Also, something I noticed that has nothing to do with the purpose of this pull request is that I was finding myself automatically hitting { TAB } to go over to the next placeholder node when placing an operation node. Whereas right now it closes entry similar to how enter works, but disables further keyboard movement, since it changes the focus of the browser to elsewhere in the window. If you press tab two more times keyboard movement comes back. Should I raise a separate issue about changing tab functionality to move between existing nodes?
@WNewhook Works perfectly for me now.
Works for me now.
@inexpensive Currently it does work the way you expect - pressing a number key makes a numeric literal node, not a variable node. Note that variables have a border and numeric literals do not. Numeric literal nodes should probably look more distinct to prevent confusion though. As for the tab thing, that's probably a good idea.
Ahh that makes sense. I just saw the orange color and thought it was a variable.
It's all good then!
It's bad styling for the numeric literals. I'm adding a blue border to indicate that it's a literal.
@inexpensive @jillhancock @christopher-rodgers here's the pull request for creating nodes with the keyboard. I couldn't figure out how to do unit testing on this after. I tested it by hand and it seems to work fine. As I recall we were planning on having everyone review everything so I marked you all as reviewers.