-
Notifications
You must be signed in to change notification settings - Fork 74
New event handler #296
base: master
Are you sure you want to change the base?
New event handler #296
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. |
$('.eventHandler').each( function() { | ||
$(this).eventHandlerView(); | ||
$(this).eventHandlerView('option', 'model', ADM) | ||
.dialog({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -1563,17 +1563,19 @@ input.screenCoordinate::-webkit-inner-spin-button { | |||
margin: 0 0.6em; | |||
} | |||
|
|||
#eventHandlerDialog { | |||
.eventHandler { | |||
width: 980px !important; /* Override computed style */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.