Add TextBox event handlers

[frenzibyte]

• Depends on Merge text addition logic into one single method #3640
• Depends on Allow deleting multiple text characters at once #3641

Allows for adding custom logic when something happens to a TextBox, will come in handy for moving audio samples from the abstract TextBox to osu!lazer's.

Also has other changes:

• Changes Commit() return type to bool for indicating that a commit actually occurred. (to avoid potentially executing custom logic on Commit() 2 times when ReleaseFocusOnCommit and CommitOnFocusLost are both set)
• Adds a bonus feature: whether the caret is currently selecting text while moving, may be useful for playing different sounds based on what state the caret is in.

[bdach]

Should it even be valid to have a text box that has both ReleaseFocusOnCommit and CommitOnFocusLost set? That just sounds broken by design, I'm not sure why someone would want to shoot themselves in the foot with that gun.

[frenzibyte]

Should it even be valid to have a text box that has both ReleaseFocusOnCommit and CommitOnFocusLost set? That just sounds broken by design, I'm not sure why someone would want to shoot themselves in the foot with that gun.

out-of-scope for this PR but definitely something to open an issue for, in fact, some of the TextBox component logic are outdated and may need to be revised and refactored at some point.

[bdach]

It is out of scope here but I'd argue it needs to be a prereq PR as the bool Commit() is a little strange in my opinion, and it'd probably end up being a simple exception throw on improper usage or something.

[frenzibyte]

It is out of scope here but I'd argue it needs to be a prereq PR as the bool Commit() is a little strange in my opinion

Why though? this could be useful to block committing on certain conditions with its sub-classes not required to duplicate the checks and just early-return if false is returned in the base method.

Might as well force that on all overridable functions.

[bdach]

this could be useful to block committing on certain conditions with its sub-classes not required to duplicate the checks and just early-return if false is returned in the base method.

I'd have to see a concrete convincing example that justifies this. Just the ability to do so doesn't convince me as it is added complexity, and I'm not sure that it's necessary.

[frenzibyte]

Should it even be valid to have a text box that has both ReleaseFocusOnCommit and CommitOnFocusLost set?

After rethinking this a bit, this should definitely be valid, this type of TextBox would commit current text when focus has been lost naturally (not by ReleaseFocusOnCommit) and release focus from the TextBox when committing naturally (not by CommitOnFocusLost).

maybe rewriting the check to commit on focus lost only if there is new committable text and remove the early-return check on Commit().

[bdach]

OK, I agree, if those behaviours are separate they do make more sense, but the double commit still does not. Still a matter for a separate PR, with proper tests and documentation.

[frenzibyte]

OK, I agree, if those behaviours are separate they do make more sense, but the double commit still does not. Still a matter for a separate PR, with proper tests and documentation.

I don't think tests are needed, it's a matter of code quality. I'm pretty sure documentation isn't even needed after separating the checks. (you'll see)

[bdach]

The publicly-exposed flags steer behaviour the stability of which is important to framework consumers, therefore I'd say it needs to be covered with tests to avoid regressions (which, FWIW, goes for as much public framework API as possible).

[frenzibyte]

@bdach I've resolved the double-commit fuzz with an internal OnTextCommitted(...) event handler for simplicity.

I've also merged addCharacter() to InsertString() and made removeCharacterOrSelection() accept removing more than one character for simplifying IME logic and centralize event invocations of text changes.

[bdach]

This PR overall is really difficult in reviewing. There are changes all over and it's hard for me to confidently arrive at the conclusion that this won't break things.

[frenzibyte]

This PR overall is really difficult in reviewing. There are changes all over and it's hard for me to confidently arrive at the conclusion that this won't break things.

OK I will split all changes into separate PRs, although I put them here just for them to make sense where they belong, but sure if it helps reviewing.

[frenzibyte]

Will leave this unchanged until the prerequisites are merged, then I'm gonna resolve conflicts afterwards.

[peppy]

Could do with some conflict resolution now.

[peppy]

Would have been nice to have some very simple test coverage of these events, but I guess it's fine as-is.

.