MVP for Multi-Line Textbox

[minetoblend]

• Closes #1618

A MVP implementation for a multi-line text box. This is still a bit messy and incomplete, but I wanted to see if this approach is generally a step in the right direction before going further.

I opted for splitting the text into words & putting them as individual DrawableSprites into a FillFlowContainer for the layout, and compute a small index from that to help with converting between draw-position and text-positions whenever the layout gets recalculated. I also attempted to extract a TextSelection class from the selection handling in TextBox, so that the selection behavior could be shared between the classes without relying on inheritance.

There's still a couple bugs that I'm aware of

• [ ] Multiple line breaks in a row get treated like a single line break.
• [ ] Line breaks at the end of the string don't show up
• [ ] Selection from mouse-down throws sometimes after the line count has changed, likely caused by a bug in the LineInfo calculation leading to null items in the Lines array

https://github.com/user-attachments/assets/bee685ba-43e7-41f0-835d-c5f7131d5934

Closes #1618

[Joehuu]

This will close https://github.com/ppy/osu-framework/issues/1618 (can't edit OP).

[frenzibyte]

This looks pretty interesting on a quick glimpse. Without diving in details, has there been any exploration or intention to merge this together with TextBox, if so, what's the end result of having that? The amount of logic copied across between TextBox and this new TextArea component feels too rough to accept as coexisting together.

I'm also curious to know what sparked this much effort out of nowhere, has this been discussed somewhere else or just pushed out like this?

[minetoblend]

has there been any exploration or intention to merge this together with TextBox, if so, what's the end result of having that? The amount of logic copied across between TextBox and this new TextArea component feels too rough to accept as coexisting together.

The TextSelection class is about as far as I went with that, though I didn't want to alter the TextBox class to make use of it too before seeing if this is the way to go about things. I think relying on inheritance is an easy way to shoot yourself in the foot with 2 components as complex as this, so the goal was to favor composition over inheritance here. Also makes it much easier to test these things in isolation. The same approach could be used for other parts of the TextBox too.

I'm also curious to know what sparked this much effort out of nowhere

For the most part just boredom, though I know that at some point I'll need a multiline text box for my typescript port of osu-framework, so I figured I might as well just make it for osu-framework first since it seems like a useful component to have. I'm also interested in eventually seeing a ModdingV2 integration in lazer (see ppy/osu-web#11113), and having a multiline text box is kind of a requirement for that.

has this been discussed somewhere else or just pushed out like this?

There's a brief exchange on discord from earlier today but nothing besides that.

[minetoblend]

Couple more thoughts on the whole thing

It'd probably make sense to add entries for the following 2 actions to PlatformAction

• Vertical text navigation (up/down keys)
• Go to text start/end (ctrl+home/end on windows)

I think naming of Select/MoveForwardLine should be reconsidered, something like SelectLineStart/End maybe. Those names are pretty ambiguous if you can also navigate up/down between lines. Without context I'd assume they're for vertical navigation.

[bdach]

This MVP is interesting but on a very cursory glance (yay for >1k diff stats! not) I have got so many open questions about it that I don't know where to begin.

• Some of the ugly warts that I just got rid of are returning (FillFlowContainer.ForceNewRow), and new ones are appearing (what is OnLayoutComputed() even for? signature looks terrible, not to mention that a virtual ComputeLayoutPositions() calling a virtual OnLayoutComputed() callback feels like an overdose of OOP)
• Inherits some of the weird breakage of flows, again (long words that exceed the width of the text box do not break a line and overflow instead, one of the worst problems remaining with text flow)
• No audio feedback, no customisable caret animations (may feel like a much of muchness but standard text box supports it), no IME, no clipboard support even? How are these going to happen, by pasting from textbox?
• I've gotten more bearish on code duplication as time goes on but how is stuff like https://github.com/ppy/osu-framework/blob/f838e4a1e3daa14f01327ca91295b68f82ee6746/osu.Framework/Graphics/UserInterface/TextArea.cs#L221-L255 not in a helper even (was lifted from TextFlowContainer)?

I dunno. It feels like there'll need to be so much follow-up work done on this to get it out of "MVP" into "usable" that I'm not even sure I have confidence this is the correct first step. If I were to say what I'd consider maybe better gun to my head style, it's probably either

• Attempt to sell TextBox for parts so that bigger chunks of that monstrosity can be reused in this thing
• Strip the part in this that does multiline, jam it in TextBox, and just jerry-rig things such that the current text box is a special case of the "generalised textbox" (i.e. all existing consumers work in "single-line mode", and anyone new that wants multiline just flips a flag or something).

Which is to say that to assess whether this is workable I'd probably have to have a go myself, and I do not foresee wanting to do that anytime soon 🤷

[minetoblend]

• Some of the ugly warts that I just got rid of are returning (FillFlowContainer.ForceNewRow), and new ones are appearing (what is OnLayoutComputed() even for? signature looks terrible, not to mention that a virtual ComputeLayoutPositions() calling a virtual OnLayoutComputed() callback feels like an overdose of OOP)

The ComputeLayoutPositions function was mainly a way to get ahold of the row data computed in FillFlowContainer but I agree it's not a great solution. In hindsight the layout should have probably just been done with a custom implementation here (probably something that directly talks to the glyph store to handle text wrapping). Something along the lines of the TextBuilder class but able to handle multiline & also providing a method of mapping between draw position & character indices.

• Inherits some of the weird breakage of flows, again (long words that exceed the width of the text box do not break a line and overflow instead, one of the worst problems remaining with text flow)

See above

• No audio feedback, no customisable caret animations (may feel like a much of muchness but standard text box supports it), no IME, no clipboard support even? How are these going to happen, by pasting from textbox?

Those are things I wanted to do after having a solid foundation that everyone agrees with, which is the main purpose of this pr.

• I've gotten more bearish on code duplication as time goes on but how is stuff like https://github.com/ppy/osu-framework/blob/f838e4a1e3daa14f01327ca91295b68f82ee6746/osu.Framework/Graphics/UserInterface/TextArea.cs#L221-L255 not in a helper even (was lifted from TextFlowContainer)?

I originally did that but the code here has slightly different handling regarding newlines which is why I moved it here. If I do layout manually (like mentioned in first paragraph) this method should become redundant anyways.

• Attempt to sell TextBox for parts so that bigger chunks of that monstrosity can be reused in this thing

I already touched a bit upon this in an earlier comment, but my TextSelection class is an attempt at a first step in that direction. Personally I'd be in favor of this approach since a lot of this stuff should work the same regardless of what kind of textbox it's put into.

• Strip the part in this that does multiline, jam it in TextBox, and just jerry-rig things such that the current text box is a special case of the "generalised textbox" (i.e. all existing consumers work in "single-line mode", and anyone new that wants multiline just flips a flag or something).

Can't say I'm on board with that one, the TextBox class is already so complex that I think this would make the whole thing nearly unmaintainable.

I'm a bit unsure about next steps here, since I prefer the the approach of splitting the textbox behavior into multiple chunks this is something I could look into. What do you say about me doing a bunch of prs attempting to split out things bit-by-bit (probably best to start with the selection since I already got started on that)? This would also be a great opportunity for adding tests for the individual parts of the textbox. The multiline textbox can get revisited afterwards.

[bdach]

What do you say about me doing a bunch of prs attempting to split out things bit-by-bit (probably best to start with the selection since I already got started on that)? This would also be a great opportunity for adding tests for the individual parts of the textbox. The multiline textbox can get revisited afterwards.

Maybe fine? I don't know. Will need to assess on a case-by-case basis. I can definitely personally say that I think there's a higher chance of that succeeding than this PR going in unchanged as I see things right now.