Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Number_input fixes #233

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Number_input fixes #233

merged 1 commit into from
Apr 19, 2024

Conversation

the-marenga
Copy link
Contributor

Sorry in advance for turning a one line fix into this, but I just kept finding unrelated bugs, while I was checking if the initial fix had any bugs and just kept fixing them.

These fixes include:

  • The initial fix for having the text input only trigger on mouse hover
  • Each number input would trigger two messages to be emitted, instead of just one. The first was explicitly triggered by shell.publish and the second one was from the text input parsing the same event one line later via self.content.on_event, which triggered it's on_input function.
  • The selection replace/delete logic had an off by one error, which just resulted in the replacement being skipped, if the selection contained the last character. The seconds parsing of the event by the textinput would then however parse the selection “correctly” and send a different message with a different value, than the first. As that value would be emitted later, it would overwrite the first value
  • This leads to any selection replacement of the last character to be accepted. Meaning, that values > max, or < min could be set, as every max would effectively have a 9 and a min of 0 as the last digit no matter what the actual max value is
  • Similarly negative max bounds, could have their value set to zero, by selecting and deleting the current value
  • The delete key could be used to set any value, by either deleting the ‘-’ char to turn negative numbers positive, or deleting the ‘.’ to turn fractions into large numbers, as these inputs were not checked at all

I was losing track of where which event part was handled, so I repackaged all of it into mainly one match and only pass keyboard events to the textinput, that are correctly checked

To make the testing for myself easier, I also added the ability to use ctra+a,v,c & accept '-' as a key input

There is still some minor jank around negative numbers and bounds that do not contain 0, but I am pretty sure these things could only be solved by not passing the event to the child/text for inserts/deletes, which would require reimplementing a bunch of selection & cursor movement logic

Closes #232

- Fix double message
- Fix keyboard input on hover
- Fix selection off by 1 error
- Fix values > max, < min to be set
- Add support for ctrl a,c,v
Copy link
Collaborator

@genusistimelord genusistimelord left a comment

Choose a reason for hiding this comment

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

looks good thank you.

@genusistimelord genusistimelord merged commit 9ed46bf into iced-rs:main Apr 19, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Numberinput requires mouse hover for keyboard input
2 participants