New event handler#296
New event handler#296xuqingkuang wants to merge 1 commit intointel:masterfrom xuqingkuang:newEventHandler
Conversation
|
Please merge the two commits into one as the first commit can't exist by itself |
|
There's a problem: When creating a new event handler, the editor doesn't have a cursor |
|
Then icon is a little big and I think two branches lightning is better than three branches lightning. |
src/js/main.js
Outdated
There was a problem hiding this comment.
Why do you create an dialog here instead of doing it in _create method of eventHandlerView?
|
@zhizhangchen The code updated by your comments, please review. There's only a issue is CodeMirror editor contents can't erase in some situation, so the editor re-initial code is exist in 'dialogopen' event handler and the 'refresh' function, it could make sure the editor is blank after selectionChanged. |
Refined the the event handler to be a standalone View, and added a icon to indicate existence of event handlers in outline View.
There was a problem hiding this comment.
This is generally a bad idea, hardcoding sizes. If my screen or window is small enough, it's impossible to access the bottom of the dialog as it does not scroll it's contents.
I'd prefer we set max, min widths and heights in hard pixels only if necessary, then use percentages for the default width and heights. Also make sure the contents of the dialog can be scrolled if the dialog it's self is too small.
|
The eventHandlerIcon.png does not really seem to fit or match the existing color scheme of the UX. I won't hold the PR becuase of this, but I suggest if we are going to try and be creative, we should follow existing color schemes. Normally, we would request new visual assets from our creative team, but, since we don't have one... ;) |
Refined the the event handler to be a standalone View, and added a icon to indicate existence of event handlers in outline View.