-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
core-splitter.html
Outdated
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.
We prefer camel case, e.g. disableSelection
|
cool, i'll make the changes |
|
@frankiefu i've made your changes let me know if there is anything else i need to do |
core-splitter.html
Outdated
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.
disableselection -> disableSelection
remove unneccessary css class setting the userSelect prop on the parent node not the body
|
@frankiefu fixed the tabbing and the missed camelCasing |
|
lgtm |
|
FWIW, you can achieve the same with preventDefault. See https://chromium.googlesource.com/chromium/src/+/master/ui/webui/resources/js/cr/ui/splitter.js#138 |
|
Sounds like this control should be built into the tracking gesture. |
|
@azakus The preventDefault needs to be done in mousedown. Calling preventDefault in trackstart did not work. I'll have to look into how trackstart is implemented. |
|
FWIW, #4 |
|
@arv I imagine that trackstart is probably too late, as it is sent asynchronously, and only after a hysteresis threshold. |
|
down doesn't work too. e.g. |
|
@frankiefu You need to use |
|
@azakus Maybe we should move this into PointerGestures repo. preventDefault in down should work, shouldn't it? |
|
Thanks @arv for the insight. Also polymer now uses polymer-gestures instead of PointerGestures. I have filed couple issues on polymer-gestures regarding preventDefault in down doesn't work and have disable selection built into tracking gesture: |
Having text become selected while moving the splitter is unwanted behavior. So lets disable text selection while the splitter is moving.