[WIP] TextEdit Widget (implements #3131) by ckaiser · Pull Request #4968 · aseprite/aseprite

ckaiser · 2025-02-01T12:04:55Z

Supersedes #4743, implements #3131.

Uses some of the skeleton of features intended for the feedback dialog just as a way to test things quickly (Help -> Feedback opens an edit) - will be removed before merging.

This is a completely new widget designed from scratch, so in theory it could replace the other multiline and text entry widgets (would need to have a readonly and one-line toggles added, but it'd be pretty simple) - but for now I need more eyes on it because I've been churning at it for a million years.

Known issues:

Caret to mouse position is a bit offset with TTF fonts - maybe? There's some strange scrolling/positioning bugs so it might be related to that, needs more testing.
Double click to select word behavior does not match Entry.
Haven't tested RTL languages yet.
Code still has some TODOs (which might be out of date).

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2025-02-01T12:09:02Z

+{
+}
+
+void FeedbackCommand::onExecute(Context* context)


warning: parameter 'context' is unused [misc-unused-parameters]

Suggested change

void FeedbackCommand::onExecute(Context* context)

void FeedbackCommand::onExecute(Context* /*context*/)

aseprite-bot · 2025-02-01T12:09:02Z

+  deleteSelection();
+}
+
+void TextEdit::copy()


warning: method 'copy' can be made static [readability-convert-member-functions-to-static]

src/app/ui/textedit.h:27:

- void copy(); + static void copy();

aseprite-bot · 2025-02-01T12:09:02Z

+  m_caret.advanceBy(clipboard.size());
+}
+
+void TextEdit::selectAll()


warning: method 'selectAll' can be made static [readability-convert-member-functions-to-static]

src/app/ui/textedit.h:29:

- void selectAll(); + static void selectAll();

aseprite-bot · 2025-02-01T12:09:03Z

+    return;
+
+  const Caret startCaret(&m_lines);
+  Caret endCaret(startCaret);


warning: variable 'endCaret' of type 'Caret' can be declared 'const' [misc-const-correctness]

Caret endCaret(startCaret); ^

this fix will not be applied because it overlaps with another fix

aseprite-bot · 2025-02-01T12:09:03Z

+
+  const Caret startCaret(&m_lines);
+  Caret endCaret(startCaret);
+  endCaret.set(m_lines.size() - 1, m_lines.back().glyphCount);


warning: local copy 'endCaret' of the variable 'startCaret' is never modified and never used; consider removing the statement [performance-unnecessary-copy-initialization]

Suggested change

endCaret.set(m_lines.size() - 1, m_lines.back().glyphCount);

endCaret.set(m_lines.size() - 1, m_lines.back().glyphCount);

aseprite-bot · 2025-02-01T12:09:03Z

+      m_pos = pos;
+    }
+
+    bool left(bool byWord = false)


warning: function 'left' is within a recursive call chain [misc-no-recursion]

bool left(bool byWord = false) ^

Additional context

src/app/ui/textedit.h:210: example recursive call chain, starting from function 'leftWord'

bool leftWord() ^

src/app/ui/textedit.h:217: Frame #1: function 'leftWord' calls function 'left' here:

if (!left()) ^

src/app/ui/textedit.h:192: Frame #2: function 'left' calls function 'leftWord' here:

return leftWord(); ^

src/app/ui/textedit.h:192: ... which was the starting point of the recursive call chain; there may be other cycles

return leftWord(); ^

aseprite-bot · 2025-02-01T12:09:03Z

+    }
+
+    // Moves the position to the next word on the left, doesn't wrap around lines.
+    bool leftWord()


warning: function 'leftWord' is within a recursive call chain [misc-no-recursion]

bool leftWord() ^

aseprite-bot · 2025-02-01T12:09:04Z

+      return true;
+    }
+
+    bool right(bool byWord = false)


warning: function 'right' is within a recursive call chain [misc-no-recursion]

bool right(bool byWord = false) ^

Additional context

src/app/ui/textedit.h:244: example recursive call chain, starting from function 'rightWord'

bool rightWord() ^

src/app/ui/textedit.h:251: Frame #1: function 'rightWord' calls function 'right' here:

if (!right()) ^

src/app/ui/textedit.h:226: Frame #2: function 'right' calls function 'rightWord' here:

return rightWord(); ^

src/app/ui/textedit.h:226: ... which was the starting point of the recursive call chain; there may be other cycles

return rightWord(); ^

aseprite-bot · 2025-02-01T12:09:04Z

+    }
+
+    // Moves the position to the next word on the right, doesn't wrap around lines.
+    bool rightWord()


warning: function 'rightWord' is within a recursive call chain [misc-no-recursion]

bool rightWord() ^

aseprite-bot · 2025-02-01T12:09:04Z

+
+    bool isWordPart(size_t pos) const
+    {
+      if (!lineObj().glyphCount || lineObj().utfSize.size() <= pos)


warning: implicit conversion 'size_t' (aka 'unsigned long') -> 'bool' [readability-implicit-bool-conversion]

Suggested change

if (!lineObj().glyphCount || lineObj().utfSize.size() <= pos)

if ((lineObj().glyphCount == 0u) || lineObj().utfSize.size() <= pos)

dacap · 2025-02-14T14:36:56Z

I'll start reviewing this today, about:

it could replace the other multiline and text entry widgets (would need to have a readonly and one-line toggles added, but it'd be pretty simple)

I'd prefer to keep both widgets separated, as the single line ui::Entry has some extra features (suffix, etc.) and combine (probably in a base class) the common functionality of both widgets.

The first thing I see is that the TextEdit widget is in app-lib (src/app/ui/textedit.h) but it should be in src/ui/textedit.h. This will limit the usage of the Strings class, SkinTheme class, etc.

A problem with Entry::showEditPopupMenu() is that it uses raw strings, I'll try to fix this first in beta branch. Using some strings in en.ini and a delegate similar to ClipboardDelegate but to translate strings from the UI. So then TextEdit can be moved to the ui-lib.

I'll continue reviewing the code as it is.

dacap · 2025-02-15T14:52:11Z

+[textedit]
+copy = &Copy
+cut = Cu&t
+paste = &Paste
+select_all = Select &All


This is a good idea, I'll try to add these strings to en.ini and start using them in ui::Entry in some way (so then you can move TextEdit to the ui-lib).

Parts of this commit are from #4968 and will be useful to reuse in the multiline TextEdit widget. Co-authored-by: Christian Kaiser <info@ckaiser.com.ar>

dacap · 2025-02-15T14:53:52Z

Added a new ui::TranslationDelegate in bc3433f

Parts of this commit are from #4968 and will be useful to reuse in the multiline TextEdit widget. Co-authored-by: Christian Kaiser <info@ckaiser.com.ar>

dacap · 2025-02-18T00:59:34Z

As the change is too big and I've already some local fixes, but cannot make a PR for this (as it includes temporal code, like the feedback command/dialog), I can cherry pick/integrate the changes from e572c83 to create the TextEdit widget and continue working from there.

dacap · 2025-02-18T01:02:05Z

Other possibility is to adjust this PR in such a way to merge it, but removing all the Feedback stuff.

Another note: Please follow the CODING_STYLE guide, mainly the one for switch/cases:

aseprite/docs/CODING_STYLE.md

Lines 83 to 87 in 3e3dd2a

    
           case 2: { 
        
             int varInsideCase; 
        
             // ... 
        
             break; 
        
           }

ckaiser · 2025-02-28T13:29:53Z

Moved to ui-lib, did a full rebase and I'm now using the TranslationDelegate - I've removed the Feedback stuff and found a nicer way to test the functionality that can actually just go into a PR, in the layer user data, seems to be working great and closes another issue while we're at it:

dacap · 2025-03-28T12:05:54Z

I'll be reviewing this one today.

dacap

There is a lot to review yet, but probably it'd be better to fix these issues, create a good first commit, and I can send you PR to your branch to continue fixing issues from there, in other case I'll make a lot of comments here and probably it's better to just continue working together in one branch.

dacap · 2025-03-28T15:53:42Z

  virtual gfx::Color calcBgColor(const Widget* widget, const Style* style);
  virtual gfx::Size calcMinSize(const Widget* widget, const Style* style);
  virtual gfx::Size calcMaxSize(const Widget* widget, const Style* style);
+  virtual gfx::Color getColorById(const std::string& id) const { return gfx::ColorNone; };


We should have a better way to use colors for the UI from the ui-lib side. Probably for ui::TextEdit we could have a ui::Style (the Widget style) for the text/face/bg, and an alternative ui::Style for the selected text (member of ui::TextEdit).

But ui-lib shouldn't know nothing about named colors/getting colors from an string ID.

I wasn't exactly sure how to do this at first but then I had the brilliant idea of just accessing the theme layers manually to get the colors, which... I don't think any other widget does so I'm pretty sure it's not right but is there any example of ui-lib stuff having multiple styles and getting the colors that way? I'm assuming we can't count on the order of the layers to be always consistent across user themes but maybe that's a requirement and this is fine.

dacap · 2025-03-28T18:19:19Z

There are other changes in .xml files to make the user data text field expansible:

diff --git a/data/widgets/layer_properties.xml b/data/widgets/layer_properties.xml
index 6c4ef4115..2793f566f 100644
--- a/data/widgets/layer_properties.xml
+++ b/data/widgets/layer_properties.xml
@@ -4,7 +4,7 @@
 <gui>
 <window id="layer_properties" text="@.title">
   <vbox>
-    <grid id="properties_grid" columns="3">
+    <grid id="properties_grid" columns="3" expansive="true">
       <label text="@.name" />
       <entry text="" id="name" magnet="true" maxsize="256" minwidth="64" cell_align="horizontal" />
       <button id="user_data" icon="icon_user_data" tooltip="@general.user_data" />
diff --git a/data/widgets/slice_properties.xml b/data/widgets/slice_properties.xml
index a683919c5..549a86f1e 100644
--- a/data/widgets/slice_properties.xml
+++ b/data/widgets/slice_properties.xml
@@ -5,7 +5,7 @@
 <gui>
 <window id="slice_properties" text="@.title" help="slices#slice-properties">
   <vbox>
-    <grid id="properties_grid" columns="3">
+    <grid id="properties_grid" columns="3" expansive="true">
       <label id="label1" text="@.name" />
       <entry id="name" maxsize="256" magnet="true" cell_align="horizontal" expansive="true" />
       <button id="user_data" icon="icon_user_data" maxsize="32" tooltip="@.user_data_tooltip" />
diff --git a/data/widgets/sprite_properties.xml b/data/widgets/sprite_properties.xml
index 19d4c5553..13fd1dd9e 100644
--- a/data/widgets/sprite_properties.xml
+++ b/data/widgets/sprite_properties.xml
@@ -4,7 +4,7 @@
 <gui>
 <window id="sprite_properties" text="@.title" help="sprite-properties">
   <vbox>
-    <grid id="properties_grid" columns="3">
+    <grid id="properties_grid" columns="3" expansive="true">
       <label text="@.filename" />
       <entry text="" id="name" maxsize="256" minwidth="64" readonly="true" cell_align="horizontal" />
       <button id="user_data" icon="icon_user_data" maxsize="32" tooltip="@general.user_data" />

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2025-08-02T03:22:15Z

+    // Check the line width and if we're more than halfway past the line, we can set the caret to
+    // the full line.
+    caret.setPos(
+      (offsetPosition.x > m_lines[caret.line()].width / 2) ? m_lines[caret.line()].glyphCount : 0);


warning: result of integer division used in a floating point context; possible loss of precision [bugprone-integer-division]

(offsetPosition.x > m_lines[caret.line()].width / 2) ? m_lines[caret.line()].glyphCount : 0); ^

aseprite-bot · 2025-08-02T03:22:15Z

+      }
+
+      for (int i = 0; i < run.glyphCount; ++i) {
+        gfx::RectF glyphBounds = run.getGlyphBounds(i).offset(gfx::PointF(0, lineStartY));


warning: variable 'glyphBounds' of type 'gfx::RectF' (aka 'RectT') can be declared 'const' [misc-const-correctness]

Suggested change

gfx::RectF glyphBounds = run.getGlyphBounds(i).offset(gfx::PointF(0, lineStartY));

gfx::RectF const glyphBounds = run.getGlyphBounds(i).offset(gfx::PointF(0, lineStartY));

aseprite-bot · 2025-08-02T03:22:15Z

+  s_timer->start();
+}
+
+void TextEdit::stopTimer()


warning: method 'stopTimer' can be made static [readability-convert-member-functions-to-static]

src/ui/textedit.h:160:

- void stopTimer(); + static void stopTimer();

aseprite-bot · 2025-08-02T03:22:16Z

+
+  void commitRunBuffer(TextBlob::RunInfo& info) override
+  {
+    ASSERT(info.clusters == nullptr || *info.clusters == 0);


warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

ASSERT(info.clusters == nullptr || *info.clusters == 0); ^

Additional context

laf/base/debug.h:67: expanded from macro 'ASSERT'

if (!(condition)) { \ ^

aseprite-bot · 2025-08-02T03:22:16Z

+
+void TextEdit::Caret::setPos(size_t pos)
+{
+  ASSERT(pos >= 0 && pos <= lineObj().glyphCount);


warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

ASSERT(pos >= 0 && pos <= lineObj().glyphCount); ^

Additional context

laf/base/debug.h:67: expanded from macro 'ASSERT'

if (!(condition)) { \ ^

aseprite-bot · 2025-08-02T03:22:16Z

+  m_pos = pos;
+}
+
+bool TextEdit::Caret::left(bool byWord)


warning: function 'left' is within a recursive call chain [misc-no-recursion]

bool TextEdit::Caret::left(bool byWord) ^

Additional context

src/ui/textedit.cpp:899: example recursive call chain, starting from function 'leftWord'

bool TextEdit::Caret::leftWord() ^

src/ui/textedit.cpp:906: Frame #1: function 'leftWord' calls function 'left' here:

if (!left()) ^

src/ui/textedit.cpp:882: Frame #2: function 'left' calls function 'leftWord' here:

return leftWord(); ^

src/ui/textedit.cpp:882: ... which was the starting point of the recursive call chain; there may be other cycles

return leftWord(); ^

aseprite-bot · 2025-08-02T03:22:16Z

+  return true;
+}
+
+bool TextEdit::Caret::leftWord()


warning: function 'leftWord' is within a recursive call chain [misc-no-recursion]

bool TextEdit::Caret::leftWord() ^

aseprite-bot · 2025-08-02T03:22:16Z

+  return true;
+}
+
+bool TextEdit::Caret::right(bool byWord)


warning: function 'right' is within a recursive call chain [misc-no-recursion]

bool TextEdit::Caret::right(bool byWord) ^

Additional context

src/ui/textedit.cpp:932: example recursive call chain, starting from function 'rightWord'

bool TextEdit::Caret::rightWord() ^

src/ui/textedit.cpp:939: Frame #1: function 'rightWord' calls function 'right' here:

if (!right()) ^

src/ui/textedit.cpp:915: Frame #2: function 'right' calls function 'rightWord' here:

return rightWord(); ^

src/ui/textedit.cpp:915: ... which was the starting point of the recursive call chain; there may be other cycles

return rightWord(); ^

aseprite-bot · 2025-08-02T03:22:16Z

+  return true;
+}
+
+bool TextEdit::Caret::rightWord()


warning: function 'rightWord' is within a recursive call chain [misc-no-recursion]

bool TextEdit::Caret::rightWord() ^

aseprite-bot · 2025-08-02T03:22:16Z

+  void paste();
+  void selectAll();
+
+  obs::signal<void()> Change;


warning: member variable 'Change' has public visibility [misc-non-private-member-variables-in-classes]

obs::signal<void()> Change; ^

dacap · 2025-10-06T14:49:29Z

I'll rebase this PR and continue this work in other branch/PR.

…3131, #4743, #4968)

dacap · 2025-10-06T15:00:06Z

We'll continue in #5455 (branch https://github.com/aseprite/aseprite/tree/multiple-lines-textbox ), I'll try to fix some problems with the selection with TTF fonts, etc.

…3131, #4743, #4968)

aseprite-bot reviewed Feb 1, 2025

View reviewed changes

dacap self-assigned this Feb 7, 2025

dacap reviewed Feb 8, 2025

View reviewed changes

Comment thread src/app/ui/textedit.cpp Outdated

dacap reviewed Feb 15, 2025

View reviewed changes

dacap added a commit that referenced this pull request Feb 15, 2025

Add new ui::TranslationDelegate to translate Entry popup items

c01dc34

Parts of this commit are from #4968 and will be useful to reuse in the multiline TextEdit widget. Co-authored-by: Christian Kaiser <info@ckaiser.com.ar>

dacap added a commit that referenced this pull request Feb 15, 2025

Add new ui::TranslationDelegate to translate Entry popup items

bc3433f

Parts of this commit are from #4968 and will be useful to reuse in the multiline TextEdit widget. Co-authored-by: Christian Kaiser <info@ckaiser.com.ar>

dacap assigned ckaiser Feb 18, 2025

ckaiser force-pushed the feature/textedit branch from 480bc0f to 4516885 Compare February 28, 2025 13:22

ckaiser force-pushed the feature/textedit branch from 4516885 to 261cb59 Compare February 28, 2025 13:30

dacap unassigned ckaiser Mar 28, 2025

dacap requested changes Mar 28, 2025

View reviewed changes

dacap assigned ckaiser Mar 28, 2025

dacap removed their assignment Mar 28, 2025

ckaiser force-pushed the feature/textedit branch 2 times, most recently from 9fba37a to 815b8c8 Compare March 31, 2025 15:13

ckaiser assigned dacap and unassigned ckaiser Mar 31, 2025

ckaiser added 3 commits August 2, 2025 00:02

*WIP* TextEdit implementation

dbd2172

Move colors to theme, style fixes, invalidate on font change

0998bec

Update to use setTextInput (maybe?)

846c9ca

ckaiser force-pushed the feature/textedit branch from 815b8c8 to 846c9ca Compare August 2, 2025 03:18

aseprite-bot reviewed Aug 2, 2025

View reviewed changes

dacap pushed a commit that referenced this pull request Oct 6, 2025

User data with multiple lines of text (new TextEdit widget) (#3130, #…

e24fabb

…3131, #4743, #4968)

dacap mentioned this pull request Oct 6, 2025

User data with multiple lines of text (new TextEdit widget) (#3130, #3131, #4743, #4968) #5455

Merged

dacap closed this Oct 6, 2025

dacap pushed a commit that referenced this pull request Oct 7, 2025

User data with multiple lines of text (new TextEdit widget) (#3130, #…

f3d36ca

…3131, #4743, #4968)

dacap pushed a commit that referenced this pull request Oct 8, 2025

User data with multiple lines of text (new TextEdit widget) (#3130, #…

fb21298

…3131, #4743, #4968)

	void FeedbackCommand::onExecute(Context* context)
	void FeedbackCommand::onExecute(Context* /context/)

	endCaret.set(m_lines.size() - 1, m_lines.back().glyphCount);
	endCaret.set(m_lines.size() - 1, m_lines.back().glyphCount);

	if (!lineObj().glyphCount \|\| lineObj().utfSize.size() <= pos)
	if ((lineObj().glyphCount == 0u) \|\| lineObj().utfSize.size() <= pos)

	gfx::RectF glyphBounds = run.getGlyphBounds(i).offset(gfx::PointF(0, lineStartY));
	gfx::RectF const glyphBounds = run.getGlyphBounds(i).offset(gfx::PointF(0, lineStartY));

Uh oh!

Uh oh!

Conversation

ckaiser commented Feb 1, 2025

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dacap commented Feb 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dacap commented Feb 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dacap commented Feb 18, 2025

Uh oh!

dacap commented Feb 18, 2025

Uh oh!

ckaiser commented Feb 28, 2025

Uh oh!

dacap commented Mar 28, 2025

Uh oh!

dacap left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dacap commented Mar 28, 2025

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

dacap commented Feb 15, 2025 •

edited

Loading