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

Widgets/text area #4995

Merged
merged 22 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
8bc9da2
Create TextArea widget, based on `gui/journal` widget
wiktor-obrebski Oct 8, 2024
55ae723
Migrate journal text-area related test to core TestArea test module
wiktor-obrebski Oct 8, 2024
e237387
Add documentation for new TextArea widget
wiktor-obrebski Oct 9, 2024
83a9a19
Add way to set text from the TextArea widget API
wiktor-obrebski Oct 9, 2024
3ce4f1f
Abandon named subviews for TextArea to avoid collisions with user code
wiktor-obrebski Oct 9, 2024
fa68bbe
Improve TextArea docs
wiktor-obrebski Oct 9, 2024
cb549ed
Add tests for TextArea undo feature
wiktor-obrebski Oct 11, 2024
ff41a5e
Add undo/redo textarea widget features tests
wiktor-obrebski Oct 11, 2024
2ed6dcb
Add clear history textarea feature
wiktor-obrebski Oct 11, 2024
58c8d9b
Add history entry (undo/redo) for TextArea API text set (:setText)
wiktor-obrebski Oct 11, 2024
cdda92e
Make TextArea on_cursor_change include old cursor
wiktor-obrebski Nov 17, 2024
a307308
Add docs about how TextArea cursor works
wiktor-obrebski Nov 17, 2024
b9422dc
Add old_text to TextArea widget text change callback
wiktor-obrebski Nov 20, 2024
51ff6b7
Drop now redundant TextArea tests version boundary
wiktor-obrebski Nov 21, 2024
1f75f37
Improve TextArea documentation
wiktor-obrebski Nov 21, 2024
ff76073
Improve TextArea documentation
wiktor-obrebski Nov 21, 2024
6384b21
Improve TextArea focus handling
wiktor-obrebski Nov 21, 2024
6047b40
Polishing TextArea subwidgets
wiktor-obrebski Nov 21, 2024
83f9df3
Remove trailing white space from docs
wiktor-obrebski Nov 22, 2024
4250bb4
Improve TextArea RST documentation structure
wiktor-obrebski Nov 22, 2024
abebac7
Improve TextArea focus
wiktor-obrebski Nov 22, 2024
c005846
Add comment about TextArea rendering
wiktor-obrebski Nov 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 134 additions & 0 deletions docs/dev/Lua API.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5513,6 +5513,140 @@ The ``EditField`` class also provides the following functions:

Inserts the given text at the current cursor position.

TextArea class
--------------

Subclass of Panel; implements a multi-line text field with features such as
text wrapping, mouse control, text selection, clipboard support, history,
and typical text editor shortcuts.

Cursor Behavior
===============

The cursor in the ``TextArea`` class is index-based, starting from 1,
consistent with Lua's text indexing conventions.

Each character, including new lines (``string.char(10)``),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Each character, including new lines (``string.char(10)``),
Each character, including newlines (``string.char(10)``),

occupies a single index in the text content.

Cursor movement and position are fully aware of line breaks,
meaning they count as one unit in the offset.

The cursor always points to the position between characters,
with 1 being the position before the first character and
``#text + 1`` representing the position after the last character.

Cursor positions are preserved during text operations like insertion,
deletion, or replacement. If changes affect the cursor's position,
it will be adjusted to the nearest valid index.

TextArea Attributes:

* ``init_text``: The initial text content for the text area.

* ``init_cursor``: The initial cursor position within the text content.
myk002 marked this conversation as resolved.
Show resolved Hide resolved
If not specified, defaults to end of the text (length of ``init_text``).
Copy link
Member

Choose a reason for hiding this comment

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

length of init_text plus one, right? (according to the Cursor Behavior text above)


* ``text_pen``: Optional pen used to draw the text.

* ``select_pen``: Optional pen used for text selection.
Copy link
Member

Choose a reason for hiding this comment

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

Please document default pens


* ``ignore_keys``: List of input keys to ignore.
Functions similarly to the ``ignore_keys`` attribute in the ``EditField`` class.

* ``on_text_change``: Callback function called whenever the text changes.
The function signature should be ``on_text_change(new_text, old_text)``.

* ``on_cursor_change``: Callback function called whenever the cursor position changes.
Expected function signature is ``on_cursor_change(new_cursor, old_cursor)``.

* ``one_line_mode``: Boolean attribute that, when set to ``true``,
disables multi-line text features and restricts the text area to a single line.
If any "\n" (``string.char(10)``) are available in the text,
they will be removed from the text
Copy link
Member

Choose a reason for hiding this comment

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

I find this phrasing a unclear for the semantics I expect. This is the behavior I'd like to see described:

  • If multiline text is pasted into the widget, newlines are removed (that is, the non-multiline SDL clipboard functions are used)
  • The Enter key is not handled by the widget as if it were included in ignore_keys


TextArea Functions:

* ``textarea:getText()``

Returns the current text content of the ``TextArea`` widget as a string.
myk002 marked this conversation as resolved.
Show resolved Hide resolved
"\n" characters (``string.char(10)``) should be interpreted as new lines

* ``textarea:setText(text)``
myk002 marked this conversation as resolved.
Show resolved Hide resolved

Sets the content of the ``TextArea`` to the specified string ``text``.
The cursor position will not be adjusted, so should be set separately.

* ``textarea:getCursor()``

Returns the current cursor position within the text content.
The position is represented as a single integer, starting from 1.
myk002 marked this conversation as resolved.
Show resolved Hide resolved

* ``textarea:setCursor(cursor)``

Sets the cursor position within the text content.

* ``textarea:scrollToCursor()``

Scrolls the text area view to ensure that the current cursor position is visible.
This is useful for automatically scrolling when the user moves the cursor
beyond the visible region of the text area.
Copy link
Member

Choose a reason for hiding this comment

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

Scrolling on cursor movement doesn't happen automatically? I'd expect this to be the default so the parent isn't required to implement on_cursor_change for a good default behavior. What are the use cases for on_cursor_change callback other than to call this function? If we had that "keep cursor visible when moving cursor" default functionality, are there any other use cases for this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we had that "keep cursor visible when moving cursor" default functionality, are there any other use cases for this function?

It is default functionality for all default manipulation of TextArea by the user.

But there is :setCursor API where we allow the API consumer to manipulate the cursor.
This function do not automatically scroll to the cursor, to allow API consumer to better control.

e.g. imagine someone want to paste a text to end of the textarea for some reason, but do not change user cursor and scroll position.

I had a similar case with gui/journal Table of Contents feature, but I do not remember now what it was exactly.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I think I understand. How about this?

     Scrolls the text area view to ensure that the current cursor position is visible.
-    This is useful for automatically scrolling when the user moves the cursor
-    beyond the visible region of the text area.
+    This happens automatically when the user interactively moves the cursor or
+    pastes text into the widget, but may need to be called when ``setCursor`` is
+    called programmatically.


* ``textarea:clearHistory()``

Clear undo/redo history of the widget.
Copy link
Member

Choose a reason for hiding this comment

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

Clear -> Clears to be consistent with the tense of other function descriptions


Functionality:

- Cursor Control: Navigate through text using arrow keys (Left, Right, Up,
and Down) for precise cursor placement.
- Fast Rewind: Use :kbd:`Ctrl` + :kbd:`Left` and :kbd:`Ctrl` + :kbd:`Right` to
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Fast Rewind: Use :kbd:`Ctrl` + :kbd:`Left` and :kbd:`Ctrl` + :kbd:`Right` to
- Move By Word: Use :kbd:`Ctrl` + :kbd:`Left` and :kbd:`Ctrl` + :kbd:`Right` to

move the cursor one word back or forward.
- Longest X Position Memory: The cursor remembers the longest x position when
moving up or down, making vertical navigation more intuitive.
- Mouse Control: Use the mouse to position the cursor within the text,
providing an alternative to keyboard navigation.
- New Lines: Easily insert new lines using the :kbd:`Enter` key, supporting
multiline text input.
- Text Wrapping: Text automatically wraps within the editor, ensuring lines fit
within the display without manual adjustments.
- Backspace Support: Use the backspace key to delete characters to the left of
the cursor.
- Delete Character: :kbd:`Delete` deletes the character under the cursor.
- Line Navigation: :kbd:`Home` moves the cursor to the beginning of the current
Copy link
Member

Choose a reason for hiding this comment

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

this should be adjacent to the word navigation feature

line, and :kbd:`End` moves it to the end.
- Delete Current Line: :kbd:`Ctrl` + :kbd:`U` deletes the entire current line
where the cursor is located.
- Delete Rest of Line: :kbd:`Ctrl` + :kbd:`K` deletes text from the cursor to
the end of the line.
- Delete Last Word: :kbd:`Ctrl` + :kbd:`W` removes the word immediately before
the cursor.
- Text Selection: Select text with the mouse, with support for replacing or
removing selected text.
Copy link
Member

Choose a reason for hiding this comment

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

this should be moved above Ctrl-U so mouse users know they can skip paying attention to the deletion keyboard shortcuts. Actually, on second thought, I'd split this list into two sections:

1st section: baseline functionality (features that players would expect from any text editor: text wrapping, mouse selection, copy/paste, undo/redo). We can probably reduce the amount of documentation dedicated to well-understood features, like Home and End. You can just mention that "the usual keyboard keys are supported, like Home, End, Backspace, Delete, etc. In addition, common mouse gestures are supported, like double click to highlight a word or triple click to highlight a line."

2nd section: keyboard power user shortcuts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not convinced, IMO we should keep all option described.
its quite think line what is "baseline" features and what now, it can be different for every user.

may proposition is to include and intro, like u proposed, but below it, in dedicated section, keep all shortcuts like now.
what do u think?

Copy link
Member

Choose a reason for hiding this comment

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

at least Text Selection in particular should be moved adjacent to the Mouse Control line so similar concepts are grouped. This list is long and feels unorganized

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Improved, what do u think about current version?

- Jump to Beginning/End: Quickly move the cursor to the beginning or end of the
Copy link
Member

Choose a reason for hiding this comment

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

should be adjacent to line navigation

text using :kbd:`Ctrl` + :kbd:`Home` and :kbd:`Ctrl` + :kbd:`End`.
- Select Word/Line: Use double click to select current word, or triple click to
Copy link
Member

Choose a reason for hiding this comment

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

should be adjacent to Text Selection

select current line.
- Select All: Select entire text by :kbd:`Ctrl` + :kbd:`A`.
- Undo/Redo: Undo/Redo changes by :kbd:`Ctrl` + :kbd:`Z` / :kbd:`Ctrl` +
:kbd:`Y`.
- Clipboard Operations: Perform OS clipboard cut, copy, and paste operations on
selected text, allowing you to paste the copied content into other
applications.
- Copy Text: Use :kbd:`Ctrl` + :kbd:`C` to copy selected text.
- copy selected text, if available
- if no text is selected it copy the entire current line, including the
terminating newline if present
- Cut Text: Use :kbd:`Ctrl` + :kbd:`X` to cut selected text.
- cut selected text, if available
- if no text is selected it will cut the entire current line, including the
terminating newline if present
- Paste Text: Use :kbd:`Ctrl` + :kbd:`V` to paste text from the clipboard into
the editor.
- replace selected text, if available
- If no text is selected, paste text in the cursor position
- Scrolling behaviour for long text build-in.
Copy link
Member

Choose a reason for hiding this comment

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

Should be adjacent to (or combined with) Text Wrapping


Scrollbar class
---------------

Expand Down
1 change: 1 addition & 0 deletions library/lua/gui/widgets.lua
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ FilteredList = require('gui.widgets.filtered_list')
TabBar = require('gui.widgets.tab_bar')
RangeSlider = require('gui.widgets.range_slider')
DimensionsTooltip = require('gui.widgets.dimensions_tooltip')
TextArea = require('gui.widgets.text_area')

Tab = TabBar.Tab
makeButtonLabelText = Label.makeButtonLabelText
Expand Down
184 changes: 184 additions & 0 deletions library/lua/gui/widgets/text_area.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
-- Multiline text area control

local Panel = require('gui.widgets.containers.panel')
local Scrollbar = require('gui.widgets.scrollbar')
local TextAreaContent = require('gui.widgets.text_area.text_area_content')
local HistoryStore = require('gui.widgets.text_area.history_store')

local HISTORY_ENTRY = HistoryStore.HISTORY_ENTRY

TextArea = defclass(TextArea, Panel)

TextArea.ATTRS{
init_text = '',
wiktor-obrebski marked this conversation as resolved.
Show resolved Hide resolved
init_cursor = DEFAULT_NIL,
text_pen = COLOR_LIGHTCYAN,
ignore_keys = {'STRING_A096'},
Copy link
Member

Choose a reason for hiding this comment

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

I believe we can take this out and default to an empty list. We seem to have fixed the issue of backtick appearing in text boxes via the INTERCEPT_HANDLED_HOTKEYS feature that I added a while back. Testing shows that we can even remove ignore_keys={'STRING_A096'}, from gui/launcher.

select_pen = COLOR_CYAN,
on_text_change = DEFAULT_NIL,
on_cursor_change = DEFAULT_NIL,
one_line_mode = false,
debug = false
}

function TextArea:init()
self.render_start_line_y = 1

self.text_area = TextAreaContent{
frame={l=0,r=3,t=0},
text=self.init_text,

text_pen=self.text_pen,
ignore_keys=self.ignore_keys,
select_pen=self.select_pen,
debug=self.debug,
one_line_mode=self.one_line_mode,

on_text_change=function (text, old_text)
self:updateLayout()
if self.on_text_change then
self.on_text_change(text, old_text)
end
end,
on_cursor_change=self:callback('onCursorChange')
}
self.scrollbar = Scrollbar{
frame={r=0,t=1},
on_scroll=self:callback('onScrollbar'),
visible=not self.one_line_mode
}

self:addviews{
self.text_area,
self.scrollbar,
}
self:setFocus(true)
Copy link
Member

Choose a reason for hiding this comment

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

this should be handled in the getPreferredFocusState override, otherwise there will be conflicts when there are multiple TextArea widgets in a single focus group

end

function TextArea:getText()
return self.text_area.text
end

function TextArea:setText(text)
self.text_area.history:store(
HISTORY_ENTRY.OTHER,
self:getText(),
self:getCursor()
)

return self.text_area:setText(text)
end

wiktor-obrebski marked this conversation as resolved.
Show resolved Hide resolved
function TextArea:getCursor()
return self.text_area.cursor
end

function TextArea:setCursor(cursor_offset)
return self.text_area:setCursor(cursor_offset)
end

function TextArea:clearHistory()
return self.text_area.history:clear()
end

function TextArea:onCursorChange(cursor, old_cursor)
Copy link
Member

Choose a reason for hiding this comment

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

to avoid polluting the public API, you can make this a local function and pass self to it. That is, change the function signature to local function textarea_onCursorChange(self, cursor, old_cursor) and change the attribute of TextAreaContent from on_cursor_change=self:callback('onCursorChange') to on_cursor_change=curry(textarea_onCursorChange, self)

and then move the textarea_onCursorChange function above where it is referenced. This is the pattern followed in other existing wigets, such as Scrollbar and Panel

Same comment for other member functions that are not part of the public API

Note this is not an issue for the text_area/* widgets since they are not top-level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this make code much harder to read and understand just to hide private API, I do not like it.
u propose a lot of local function that are in reality hided class function.
also, currying, which include another layer of understanding for a new dev.

maybe instead we should incorporate e.g. Python "private" functions by convention?
e.g. TextArea:__onCursorChange. This make code still quite simple to read and it will be very hard to have an accidental naming collision.

Copy link
Member

Choose a reason for hiding this comment

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

I'd be ok with something like that. @lethosor do you have a preference?

local x, y = self.text_area.wrapped_text:indexToCoords(
self.text_area.cursor
)

if y >= self.render_start_line_y + self.text_area.frame_body.height then
self:updateScrollbar(
y - self.text_area.frame_body.height + 1
)
elseif (y < self.render_start_line_y) then
self:updateScrollbar(y)
end

if self.on_cursor_change then
self.on_cursor_change(cursor, old_cursor)
end
end

function TextArea:scrollToCursor(cursor_offset)
if self.scrollbar.visible then
local _, cursor_liny_y = self.text_area.wrapped_text:indexToCoords(
Copy link
Member

Choose a reason for hiding this comment

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

is cursor_liny_y supposed to be cursor_line_y?

cursor_offset
)
self:updateScrollbar(cursor_liny_y)
end
end

function TextArea:getPreferredFocusState()
return self.parent_view.focus
end

function TextArea:postUpdateLayout()
self:updateScrollbar(self.render_start_line_y)

if self.text_area.cursor == nil then
local cursor = self.init_cursor or #self.init_text + 1
self.text_area:setCursor(cursor)
self:scrollToCursor(cursor)
end
end

function TextArea:onScrollbar(scroll_spec)
local height = self.text_area.frame_body.height

local render_start_line = self.render_start_line_y
if scroll_spec == 'down_large' then
render_start_line = render_start_line + math.ceil(height / 2)
elseif scroll_spec == 'up_large' then
render_start_line = render_start_line - math.ceil(height / 2)
elseif scroll_spec == 'down_small' then
render_start_line = render_start_line + 1
elseif scroll_spec == 'up_small' then
render_start_line = render_start_line - 1
else
render_start_line = tonumber(scroll_spec)
end

self:updateScrollbar(render_start_line)
end

function TextArea:updateScrollbar(scrollbar_current_y)
local lines_count = #self.text_area.wrapped_text.lines

local render_start_line_y = (math.min(
Copy link
Member

Choose a reason for hiding this comment

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

the outer parentheses are unnecessary and don't add clarity

#self.text_area.wrapped_text.lines - self.text_area.frame_body.height + 1,
math.max(1, scrollbar_current_y)
))

self.scrollbar:update(
render_start_line_y,
self.frame_body.height,
lines_count
)

if (self.frame_body.height >= lines_count) then
render_start_line_y = 1
end

self.render_start_line_y = render_start_line_y
self.text_area:setRenderStartLineY(self.render_start_line_y)
end

function TextArea:renderSubviews(dc)
self.text_area.frame_body.y1 = self.frame_body.y1-(self.render_start_line_y - 1)
myk002 marked this conversation as resolved.
Show resolved Hide resolved

TextArea.super.renderSubviews(self, dc)
end

function TextArea:onInput(keys)
if (self.scrollbar.is_dragging) then
return self.scrollbar:onInput(keys)
end

if keys._MOUSE_L and self:getMousePos() then
self:setFocus(true)
end

return TextArea.super.onInput(self, keys)
end

return TextArea
Loading