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

Fix MaskedInput selection replace with invalid value #5494

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

JonathanPlasse
Copy link

@JonathanPlasse JonathanPlasse commented Jan 24, 2025

Please review the following checklist.

This PR implements the MaskedInput.replace() method by using the already existing MaskedInput.insert_text_at_cursor() which replace the characters following the cursor position.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md (where appropriate)

@TomJGooding
Copy link
Contributor

TheMaskedInput will select all text on focus by default, but this isn't obvious until you add the highlighting.

Wouldn't this PR only replace a single character, rather than the whole selection as you would expect?

@JonathanPlasse
Copy link
Author

You are right

@JonathanPlasse
Copy link
Author

JonathanPlasse commented Jan 25, 2025

Wouldn't this PR only replace a single character, rather than the whole selection as you would expect?

Fixed

@TomJGooding
Copy link
Contributor

Unfortunately, I think fixing the MaskedInput to handle text selection is going to be more complicated.

For example, try running the credit card example in the docs with the changes in this PR. After entering some numbers to complete the input, try selecting some text using the mouse (it might help adding the highlighting) then enter a number. This will crash if the selection included a separator as the value does not match the template.

@JonathanPlasse JonathanPlasse force-pushed the fix-masked-input-selection-replace branch from 9f89228 to d65641f Compare February 1, 2025 07:14
@JonathanPlasse
Copy link
Author

This is now fixed.
The replace now behave like this:

  • if the replace text is the same length as the selection (without separator), it replace exactly the selection
  • if the replace text is greater, the replace text will be cut to fit within the selection
  • if the replace text is lesser, the remaining selection text will be deleted
  • if the selection starts on a separator, the replace text start after it

@JonathanPlasse JonathanPlasse force-pushed the fix-masked-input-selection-replace branch from d65641f to a40592c Compare February 1, 2025 07:24
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.

MaskedInput ValueError Value does not match template
2 participants