Skip to content

Conversation

@wiktor-obrebski
Copy link
Contributor

This PR migrate commonly used EditField widget to be based on TextArea component.
Its inherits all TextArea features in one line mode, keeping original interface of EditField widget.

Also, I polished one line mode of TextArea to display cp437 character () instead of remove them.

screen

@wiktor-obrebski wiktor-obrebski force-pushed the widgets/edit-field-to-text-area branch from 048658b to de678b1 Compare January 15, 2025 06:59
myk002
myk002 previously approved these changes Jan 16, 2025
Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

approved, but holding off on merge until after 50.15-r2 is tagged since we're so close to release

expect.eq(1, e.cursor, 'cursor should be at beginning of string')
e:onInput{CUSTOM_CTRL_RIGHT=true}
expect.eq(6, e.cursor, 'goto beginning of next word')
expect.eq(5, e.cursor, 'goto end of current word')
Copy link
Member

Choose a reason for hiding this comment

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

I think I was mimicking readline behavior before, but I agree with this change since it matches browser edit field behavior.

@myk002
Copy link
Member

myk002 commented Jan 18, 2025

From testing, there is an issue with focus setting in gui/launcher. If you do a history search (via Alt-s or by clicking on the search widget) and then right click or hit esc to go back to editing the commandline, no widget has focus and key input is lost. Are the onunfocus handlers being run correctly?

Also, for activatable edit fields that you can click to set focus and start editing, like the blueprint name in gui/blueprint, the existing text will become highlighted from the end of the word to the place where you clicked. It would make more sense to highlight the entire string on activation or not highlight anything and ignore the first click, leaving the cursor at the end of the string.

However, if we highlight everything on focus, we should fix another (existing) behavior I noticed when I double clicked to highlight a word, then hit an arrow key. Currently, the cursor gets positioned relative to the beginning of the word when you do this. If a word is highlighted and I hit the left arrow, I'd expect the cursor to be at the beginning of the word, not one position to the left of the word. If a word is highlighted and I hit the right arrow key, I expect the cursor to be just after the last letter of the word, not one position to the right from the first letter of the word.

@myk002 myk002 dismissed their stale review January 18, 2025 17:28

testing shows some issues that need to be fixed first

@wiktor-obrebski wiktor-obrebski force-pushed the widgets/edit-field-to-text-area branch 2 times, most recently from f0da058 to 9028795 Compare January 19, 2025 09:49
TextArea should not consume up/down chars in one line mode
@wiktor-obrebski wiktor-obrebski force-pushed the widgets/edit-field-to-text-area branch from 9028795 to 8e3a79e Compare January 19, 2025 09:49
@myk002
Copy link
Member

myk002 commented Jan 19, 2025

Focus is fixed for gui/launcher and the partial-highlight-on-activate issue in gui/blueprint is fixed, but now I'm seeing a few new issues:

  • in gui/launcher you can no longer iteratively search the history. Repro steps:
    • type something that will match multiple elements in your history
    • hit Alt-s to search. first match shows on the commandline
    • hit Alt-s again. in HEAD, the next matched commandline in the history will show. with this PR, the matched commandline doesn't change
  • in gui/blueprint, the edit field contents can revert to an incorrect string. Repro steps:
    • start gui/blueprint
    • edit the name field to something unique and hit enter to commit
    • re-select the name field and cancel out
    • expected: the string does not change; actual: the string is cleared to the default initial value
    • I If you commit a new string again, then re-activate and cancel, you'll get the first string you entered up at step 2. I suspect the onfocus/onunfocus behavior is different from the original EditField such that gui/blueprint is not getting the events at the same time as before.

@myk002 myk002 merged commit f5e17bb into DFHack:develop Jan 20, 2025
14 checks passed
myk002 added a commit that referenced this pull request Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants