GraphiteEditor
DropdownInput preview support and ColorButton history improvements
As follow ups of https://github.com/GraphiteEditor/Graphite/pull/1584#issuecomment-1926521666
sry, it's still work in progress, not ready even for reviewing. I should open this as a draft pr...
Ah, yes, good to open PRs early but just please mark them as a draft. I usually don't poke around draft PRs. But just make sure to ping me once you're ready for review, and switch it to non-draft. Thanks :)
@Keavon Actually, I opened this pr actually would like to confirm two user experience behaviors first.
Also, same for TextInput and TextAreaInput that should send updates as the user types but only commits when that currently happens.
Does it mean user type in TextInput in right panel, would like the text change according to in the canvas as user typing.(Right now it only changes when the TextInput is unfocus.)
So if you edit a color, once you release the mouse after dragging any of the sliders, it should send a commit at that time.
Does it means these sliders? (Right now these sliders do not save any history steps separately, only save a history step after close Color Panel)
Does it mean user type in TextInput in right panel, would like the text change according to in the canvas as user typing.(Right now it only changes when the TextInput is unfocus.)
If I'm interpreting your question correctly, I think you're asking if text created with the Text tool should be re-rendered as the user types in the Properties panel? My answer to that is: maybe, but only if the performance isn't too bad. There is also #1589 currently open so let's avoid implementing that right now in case it causes conflicts. I'm mostly just interested in having the text widget send its real-time text but letting the user of the text, like the Properties panel for the Text node, decide if it wants to use the real time version or the committed version (which is what is used right now).
Does it means these sliders? (Right now these sliders do not save any history steps separately, only save a history step after close Color Panel)
- A, B, C: Should update when the user is finished dragging
- D: Should update when the user commits a new typed value by hitting Enter, clicking out of the box, or closing the entire color picker popover by moving the mouse far away from it
- E: Since this can be dragged or typed in, the previous two bullet points apply to this
- F: Should update immediately upon being clicked or picking a color with the eyedropper
All of the above should send the history update as described, but duplicates should be avoided if the user committed a change that resulted in a value that's the same as when they started to edit that value.
@Keavon could have a code review when you have time.
This PR is mainly about Dropdown preview ability and Color Button related history actions (All actions in below image).
For TextInput widget, I haven't handled in this PR. I found unless we really need send updates on every typing by the users, current text widget has satisfied enough right now. Maybe we could return to this if we meet some concrete issues in future 🤔
I'm marking this as a draft while awaiting the requested changes. Please un-mark as draft and also ping me when ready for a second review. Thanks!
Hi, I'm just checking up on how things are going with this feedback round. Thanks :)
sry for the late, a little busy these two weeks. I'll look forward to see if I could find some time next week.
| 📦 Build Complete for 9dce4d54b511f517525d92def9c67edbf5b0ec3a |
|---|
| https://ff05ca5f.graphite.pages.dev |
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
I rebased this for you, and now strangely I'm getting fatal console errors when I hover over the dropdown menu entries. Can you please help investigate? I committed some debug instrumentation which points out that it's getting a value of -1 where it should be able to fit as a u64, which is weird since it also gets sent from the JS code as a number that's 0 or greater. Maybe you can help figure out the issue? Thanks.
| 📦 Build Complete for 0e8d5f2540530260b60c6df04b5f944a18c2b181 |
|---|
| https://59efb20a.graphite.pages.dev |
it's related to these codes, if I comment these out, it could work. https://github.com/GraphiteEditor/Graphite/pull/1598/commits/d6e357f5bd0d37103034ec7f1043f977fd815621
but I still don't know how fix this. I'm not familiar with MenuList yet. It should be related to this PR: https://github.com/GraphiteEditor/Graphite/pull/1499
Ah, that's a good hint, thank you for investigating and narrowing it down to that! If you'd like me to take it from here, I can do that in the next couple days. Or if you'd like to keep digging and find the solution, please feel free to do that as well.
Pardon the very long delay, this required rebuilding some mental context and I was quite busy with things unrelated to this. Thank you for helping make this happen, and your patience in my finally returning to merge it.