All ways of creating node (+ button, dragging out edges) #8341
Conversation
MichaelMauderer
added
-gui
CI: No changelog needed
MichaelMauderer
force-pushed
the
wip/MichaelMauderer/gui2/All-ways-of-creating-node-(+-button,-dragging-out-edges)
branch
from
705c06b to
9da34fc
Compare
| </template> | ||
|
|
||
| <script> | ||
| export default { |
There was a problem hiding this comment.
This is an atypical component definition. Use <script setup lang="ts">.
| @@ -0,0 +1,55 @@ | |||
| <template> | |||
| <div class="parent"> | |||
There was a problem hiding this comment.
looks like this is an unused class, and doesn't match component name. Use PlusButton class here, as it makes tests and debugging easier if we keep that convention everywhere.
| singleClickTimer = setTimeout(() => { | ||
| clickCount = 0 | ||
| // If within proper time range, consider it as fast click | ||
| if (Date.now() - lastClickTime >= timeBetweenClicks) { | ||
| onClick() | ||
| } | ||
| lastClickTime = Date.now() | ||
| }, timeBetweenClicks) |
There was a problem hiding this comment.
This logic seems wrong to me, as lastClickTime is set within the timer callback only. Isn't it true that if the timer callback was allowed to execute in the first place, then the event wasn't a double-click?
Also, delaying a click handler just to allow for doubleclick action is a really bad UX. The doubleclick can only really be used in cases where the first click handler can be executed immediately anyway, e.g. allowing the node to be selected, only then to start editing it after second click. In your case, I believe it would mean first click starts edge drag, but then subsequent doubleclick simply "overrides" that action with edge creation and predefined position.
Also for handling that, you can use builtin events instead of relying on your own timer. For example, a click provides a detail field, which tells you about which click in a sequence it is (1 for first, 2 for doubleclick etc.). There is also a dedicated dblclick event, though I'd rather use the detail field instead of two handlers (because click will fire again together with dblclick).
There was a problem hiding this comment.
I've removed the delay for now as for my use case it is not relevant either way.
It seems detail does not work for pointerdown events, as it is always zero. So I left this to support usage in those events.
…ting-node-(+-button,-dragging-out-edges) # Conflicts: # app/gui2/src/components/GraphEditor.vue # app/gui2/src/components/GraphEditor/GraphEdges.vue # app/gui2/src/stores/graph/index.ts
MichaelMauderer
force-pushed
the
wip/MichaelMauderer/gui2/All-ways-of-creating-node-(+-button,-dragging-out-edges)
branch
from
87c1013 to
d6cc6af
Compare
MichaelMauderer
added
the
CI: Ready to merge
mergify
bot
deleted the
wip/MichaelMauderer/gui2/All-ways-of-creating-node-(+-button,-dragging-out-edges)
branch
Pull Request Description
Closes #8054
Peek.2023-11-20.13-50.webm
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.