Closed mdietz closed 11 years ago
If you're going to introduce mousetrap, is it possible to get rid of jquery.hotkeys? We have a million vendor js deps.
remove trailing whitespace from style.styl and help.mustache
Issues after trying it out:
This feature is really great. Can't wait to get it polished up and merged :)
Also, in the interest of getting this feature finished, I would be ok with axing the markdown cheat sheet portion of the help dialog for now. Then we can merge this sooner and we'll take care of that feature as part of #61.
I'm going to add the defocus keybinding, restyle the keyboard shortcuts part of the help page, and fix the shift_n keyboard shortcut notation in the help page. Very much in favor of axing the markdown help until we can make a polished version.
The help dialog looks similar to the message preview dialog, but it's different.
I can take care of these things when I get a chance.
I'd like the shift+n
/shift+p
shortcuts to wrap around (e.g., shift+p
on the first channel goes to the last one). Thoughts?
I kind of like the fact that you can type with the markdown cheatsheet up, but I can see why you would want to make the more consistent.
Paraphrasing from Prat discussion:
If you can interact with the background, it introduces too many corner cases. If the window is small, or if the dialog becomes big, it could cover up (for example) the input box, which could be confusing.
Also, it allows for the possibility of opening multiple modals at once. You can do that right now actually -- open the help dialog and then open preview.
Disallowing interaction with the background while the modal has focus really limits the weird corner cases, and I think that 'click elsewhere to make it go away' is expected behavior. If you look at other examples of these kinds of dialogs on the interwebs, you'll see the same thing:
I'm with @cespare on this. The modals should take focus and disable interaction with the rest of the page.
okay, you've convinced me
Lots of styling tweaks added, finally figured out why modals aren't dismissing correctly (and fixed it for our current modal dialogs), and addressed @cespare 's tweaks in https://github.com/bkad/oochat/pull/57#issuecomment-12688993.
This feature should be ready to merge.
Nice, you did the circular channel switching.
I still have a few style tweaks incoming, I'll push in a bit.
I'd also like to fix the following corner case before we merge: right now, when a modal is open, it's still possible to interact with the app via the shortcuts. I'd like to either disable shortcuts completely when inside a modal, or make any keypress dismiss the modal (and not trigger a shortcut).
OK, I added a styling commit. Let me know what you guys think.
I'm ready for merge on this whenever. I want to experiment with changing our use of bootstrap modals (and fix the bug I mentioned above) but for now, I think I'm ready to see this merged. It should be useful and reasonably polished as is.
I like the style tweaks.
Support for simple keyboard shortcuts including ?:help, j/k: next/prev msg, shift+n/p: next/prev channel.