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

Basic text input #495

Merged
merged 28 commits into from
May 18, 2022
Merged

Basic text input #495

merged 28 commits into from
May 18, 2022

Conversation

darrenburns
Copy link
Member

@darrenburns darrenburns commented May 11, 2022

Goals for this PR

  • A single line text input widget
    • Placeholder text which appears when content is empty
    • Text insertion
    • Text deletion
    • Cursor movement left and right
    • Integration with focus system (tabs should not be consumed)
    • Events fired when text input value changes or is submitted (enter key press)
    • Basic scrolling based on cursor movement (and text insertion/deletion if appropriate)
    • Focus styling should work when focus/blur happens, be colourblind friendly
    • A simple API for text editing that can be extended to support more advanced editing functions, but supports the functions required for single line text input.
    • A simple demo application which lets us exercise text input, and get a feel for ergonomics of the widget etc. (see sandbox/input.py)
    • Keybinds
      • Home and end support
      • Left and right
      • Delete forward and back
    • Tests for the text editor backend class

Non-goals

  • Multiline input - I will implement a very naive basic multiline edit feature to help with modelling the API etc, but my goal is not to deliver a polished multiline input widget.
  • Efficiency of text storage - no piece tables, ropes, etc. to see here :) with the focus being on single line here, it doesn't make sense. The text storage will be a simple string, but abstracted away into a class to make it easier to swap out going forward.
  • Clicking somewhere makes the cursor go to that location - not MVP, will reserve this for later

from textual.widget import Widget


class TextInputBase(Widget):
Copy link
Member Author

Choose a reason for hiding this comment

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

Trying to think ahead a little here by extracting some common keybinding/text navigation stuff into a base class. Not sure if it will work out, but that's my thinking.

@darrenburns darrenburns force-pushed the text-input branch 2 times, most recently from 85ba2c7 to f6002e7 Compare May 17, 2022 10:26
@darrenburns darrenburns marked this pull request as ready for review May 17, 2022 10:31
@darrenburns
Copy link
Member Author

Having some problems with Text wrapping where typing multiple words separated by spaces can sometimes cause the second word to disappear. I'm assuming it's folding on to the next line. Have tried playing with overflow and no_wrap but no luck

Copy link

@olivierphi olivierphi left a comment

Choose a reason for hiding this comment

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

Amazing work! 😮
I just have a few very picky comments on things that don't matter, that you're totally free to ignore :-)

src/textual/_text_backend.py Outdated Show resolved Hide resolved
src/textual/_timer.py Show resolved Hide resolved
src/textual/keys.py Outdated Show resolved Hide resolved
src/textual/widgets/text_input.py Outdated Show resolved Hide resolved
src/textual/widgets/text_input.py Show resolved Hide resolved
src/textual/widgets/text_input.py Outdated Show resolved Hide resolved
def get_content_height(
self, container_size: Size, viewport_size: Size, width: int
) -> int:
return self._editor.content.count("\n") + 1 + 2

Choose a reason for hiding this comment

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

could be nice to add a quick comment that explains why we add 1 and 2 ? :-)

Copy link
Member Author

@darrenburns darrenburns May 17, 2022

Choose a reason for hiding this comment

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

Aha, but that assumes that I know why I added 1 and 2! ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Going to leave this for now since it's just for my naive/hacky little multiline editor thing, not intended to be production ready really :)

Copy link
Collaborator

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

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

I like the direction this is going.

Haven't been too pedantic re docstrings etc. since I know this is the first part of a feature branch.

src/textual/widgets/text_input.py Show resolved Hide resolved
src/textual/keys.py Outdated Show resolved Hide resolved
src/textual/widgets/text_input.py Outdated Show resolved Hide resolved
src/textual/widgets/text_input.py Outdated Show resolved Hide resolved
src/textual/widgets/text_input.py Show resolved Hide resolved
src/textual/widgets/text_input.py Outdated Show resolved Hide resolved
src/textual/widgets/text_input.py Outdated Show resolved Hide resolved
@@ -200,6 +200,9 @@ class Keys(str, Enum):
ShiftControlEnd = ControlShiftEnd


KEY_BINDINGS = Keys._value2member_map_.values()

Choose a reason for hiding this comment

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

👌

@darrenburns
Copy link
Member Author

@willmcgugan I've fixed the issue where scrolling would break on resize (since the "visible window" into the editor content wasn't being adjusted previously), and also I had missed a no_wrap on the placeholder text in one case.

@darrenburns darrenburns merged commit 89fd395 into css May 18, 2022
@darrenburns darrenburns deleted the text-input branch May 18, 2022 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants