search

ppy / osu-framework

A game framework written with osu! in mind.
MIT License
1.66k stars 417 forks source link

Add `appendOnly` mode to `TextBox` #4741

Open jthistle opened 3 years ago

jthistle commented 3 years ago

Rationale

The text input used to search beatmaps recently received a change made in this commit in the main osu!lazer repo. Essentially, this means that you can no longer use the delete key based on the assumption that "the caret should always be at the end of text". Of course, you can move the caret by clicking, making using the text box a little unintuitive. For example, if I click to move the caret to a position where I want to delete the following text, I find that I can't do that. Instead of just 'assuming' that the caret won't be moved by the user to inside the text, why not actually prevent the user from doing this?

This is already the case in stable, where there is no caret in the beatmap search.

Suggestion

Add a new flag to TextBox restricting the caret to allow only appending and deleting from the end of the line of text. Potentially, this could also replace the flag HandleLeftRightArrows, since this is also a property of TextBox and is only used in this specific case.

If I can gain some approval for this idea, I would also like to implement it if possible - it's the right kind of size for a new contributor, I guess :)

peppy commented 3 years ago

I'm really not sure this should be at a framework level. Sounds like something to be done osu!-side if at all.

jthistle commented 3 years ago

Hi, thanks for the quick response. HandleLeftRightArrows is already at framework level though, why would this be different?

peppy commented 3 years ago

If anything I'd say that should also be removed. It's a very dated code path which we'd probably do differently these days. It also dates back to a time where textbox was less flexible than it is now.

jthistle commented 3 years ago

Ok, cool. So that too is something that ought to be implemented at lazer level.

But, if we ignore whether this is a matter for the framework or for lazer, would you agree with the design decision to make the beatmap song search an line-end-only editable textbox? (i.e. the caret is fixed to the line end)

peppy commented 3 years ago

Unsure. I’d rather find a way to make it fully traversable instead. Need to experiment with that to give an answer.

jthistle commented 3 years ago

Cool. The main problem is that, 1) the beatmap search is focus-hungry, and 2) the keybindings normally used for text traversal are used for navigating beatmaps. This makes it impossible for the beatmap search textbox to be conventionally traversable without sacrificing one of these things.

It would seem to me that the only solutions in this case would be either to make the beatmap search not focus-hungry, or to make it line-end-only like in stable.

ekrctb commented 3 years ago

@jthistle Just to be clear, the linked commit https://github.com/ppy/osu/commit/1bac471b49bf9cc34154a3e182c5945ba6284d77 was not supposed to change the behavior. I tested and couldn't find a behavior change. The forward delete action has been blocked by the search text box since https://github.com/ppy/osu/pull/5372.

jthistle commented 3 years ago

@ekrctb apologies, I referenced that commit because when I git blamed that line, it was the one that came up... I realise now that that doesn't necessarily mean you changed it in a meaningful way in that commit. Either way, that line is still part of (or at least a symptom of) this problem.