Skip to content

Ports Textbox2d to v2 architecture#1134

Open
medha-14 wants to merge 11 commits intofury-gl:v2from
medha-14:textbox2d
Open

Ports Textbox2d to v2 architecture#1134
medha-14 wants to merge 11 commits intofury-gl:v2from
medha-14:textbox2d

Conversation

@medha-14
Copy link
Copy Markdown

@medha-14 medha-14 commented Feb 28, 2026

Issue #1109

image

@medha-14 medha-14 changed the title Textbox2d Ports Textbox2d to v2 architecture Feb 28, 2026
@medha-14
Copy link
Copy Markdown
Author

I’ve made the required changes to the Textbox2D element and added more comprehensive tests, following the approach used for LineSlider2D. Could you please review the changes and let me know if anything else is needed?

@medha-14
Copy link
Copy Markdown
Author

medha-14 commented Mar 2, 2026

@ganimtron-10 @maharshi-gor Whenever you get the chance, could you please take a look? I’d really appreciate your feedback so I can make the necessary improvements.

@ganimtron-10
Copy link
Copy Markdown
Contributor

Hi @medha-14 ,
Thanks for the PR!
Will review this PR sometime later today or tomorrow. Meanwhile can you add a demo/example so its quicker to test and verify.

@medha-14
Copy link
Copy Markdown
Author

medha-14 commented Mar 8, 2026

@ganimtron-10 Thank you for the reminder. I have added the example now. Whenever you have the time, I would really appreciate it if you could take a look.

@ganimtron-10 ganimtron-10 self-requested a review March 11, 2026 20:43
Copy link
Copy Markdown
Contributor

@ganimtron-10 ganimtron-10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @medha-14 ,

You are on right tracks but found few initial hiccups while playing with the demo.

  1. Bold and Italic example is not exactly working as expected (might need to check TextBlock2D)
Image
  1. When I try typing into the TextBox same key is registered multiple times
Image

I have also added few comments below, PTAL.

Comment thread fury/ui/tests/test_elements.py Outdated
Comment thread fury/ui/core.py Outdated
Comment thread fury/ui/core.py
Comment thread fury/ui/elements.py Outdated
Comment thread fury/ui/elements.py Outdated
Comment thread fury/ui/elements.py Outdated
Comment thread fury/ui/elements.py Outdated
Comment thread fury/window.py
Comment thread fury/window.py
Comment thread fury/window.py Outdated
@maharshi-gor
Copy link
Copy Markdown
Contributor

Hey @medha-14 are you still planning to work on this?

@medha-14
Copy link
Copy Markdown
Author

medha-14 commented Apr 2, 2026

Hey @medha-14 are you still planning to work on this?

Yes, I am just in the process of addressing the reviews , sorry for the delay.

@medha-14
Copy link
Copy Markdown
Author

medha-14 commented Apr 3, 2026

Hi @ganimtron-10 and @maharshi-gor, thanks a lot for the detailed l review. I’ve addressed all the comments and also fixed the issues related to multiple key presses and italics.

Could you please take another look and let me know if anything else needs to be improved?

@maharshi-gor
Copy link
Copy Markdown
Contributor

Hey @medha-14 the CIs are failing please check those.

@medha-14
Copy link
Copy Markdown
Author

medha-14 commented Apr 3, 2026

Hey @medha-14 the CIs are failing please check those.

@maharshi-gor could you please run the checks again, I have fixed the issue now. Thanks :)

@ganimtron-10
Copy link
Copy Markdown
Contributor

Hi @medha-14, while skimming through the code, I didnt feel the implementation to be as expected.

Main thing that didnt felt right was the active_ui setup, currently when the left mouse is pressed we directly set the textbox to be active_ui which should not be the case because moving forward we might have a hierarchy of things and setting active_ui might be complex.

Ideally how it should work is UI flows between mainly three states (for understanding simplicity)
a. Normal b. Hot c. Active

Normal is the default state of the UI.
Hot is when the user might be trying to active the UI ie. when he hover over the UI area or entering into it.
Active is when the UI was hot and then the user started interacting with it ie. he hovered onto the UI area and then clicked it.
(Great Video for more context: https://youtu.be/Z1qyvQsjK5Y?si=kg9pYfwSdNQtQNO8)

Currently we are directly setting it into active_ui which works now but cause us problem down the line.
If you could fix this flow and add the hot_ui logic on entry and disable it on exit and then if the UI is interacted then setting it to active needs to be done. Also all this logic will not be surely not be into the individual textbox, it will be in a combination of core UI, windows.py and ui/context.py when and where required

@medha-14
Copy link
Copy Markdown
Author

@ganimtron-10 Thanks for sharing the resource , this is really helpful. I understand your point about the UI state flow and how directly setting active_ui on mouse press could cause issues as the system scales.

I’ll go through the implementation again, to add the proper hot_ui handling on hover or entry and exit, and update the flow so that active_ui is only set after the correct transition.

I’ll push a fix soon, thanks again for the detailed feedback!

@maharshi-gor
Copy link
Copy Markdown
Contributor

Hey @medha-14 any new updates?

@medha-14
Copy link
Copy Markdown
Author

medha-14 commented Apr 21, 2026

@ganimtron-10 @maharshi-gor I have added the suggested changes, please have a look and let me know if anything else is needed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants