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

TextArea #447

Closed
wants to merge 29 commits into from
Closed

TextArea #447

wants to merge 29 commits into from

Conversation

Konstantin8105
Copy link

Create for #64

  • Adding/removing characters
  • Navigating the text
  • Graphemes support
  • Test for finding corner cases
  • Add demos source
  • Minimal change into TextView
  • No color
  • No regions
  • No Form interface

if unicode.IsLetter(r) ||
unicode.IsNumber(r) ||
unicode.IsMark(r) ||
unicode.IsSpace(r) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This filtering is too strict - it blocks user from entering any special characters, eg !"£$%^&*(){}~@:<>? and more. I think it could be reduced to just "unicode.IsGraphic", so that it only blocks control sequences.

Copy link
Author

Choose a reason for hiding this comment

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

I agree with you.

@jpeletier
Copy link

@Konstantin8105 Given the pace at which reviews go in this repo, I'd suggest you move this widget to your own staond-alone repo under your control so we can begin using this and you can get PRs for improvements. I did this with my widget, winman so it would be available immediately to everybody.

@Konstantin8105
Copy link
Author

@jpeletier In according to LICENSE, you can merge that widget in your project without any link on myself. Feel free to coping, using, and ...

@jpeletier
Copy link

@Konstantin8105 Yes, thank you for a permisive license, but I would like to avoid copying your code if I can better link to it and everyone shares the same to improve it together.

This repo is quite "slow". If you keep this in your repo, we can probably evolve your TextArea widget faster together.

@ghost
Copy link

ghost commented Oct 25, 2020

This is actually a very very useful addition. Is there any reason it cannot be merged into master. Just trying to get a picture on timeline as this is also what I need.

@buzzdan
Copy link

buzzdan commented Dec 30, 2020

watching this one as well...

@skanehira
Copy link

This is very useful!
I waiting and I hope TextArea added

@l13t
Copy link

l13t commented Jan 7, 2021

Also waiting for merge of this PR.

@brlbil
Copy link

brlbil commented Mar 13, 2021

I have been waiting for this future to start a personal project. So I appreciate it if it can be merged asap.

@rivo
Copy link
Owner

rivo commented Mar 13, 2021

I know this will be disappointing to some of you but it's unlikely I will merge this PR. There are a whole bunch of reasons but the two major reasons are: I don't think TextArea should be based on TextView because it would inherit all the shortcomings of TextView, mainly its inability to handle large texts as well as its non-optimal performance. The second major reason is that this PR has a bug at its core that will not be trivial to fix: Runes are not Unicode graphemes. A grapheme can consist of multiple runes. (A lot of emojis, e.g. flags, consist of multiple runes.) This will lead to bugs; I've already seen and fixed many related bugs.

Please fill out #578, TextArea is one of the features you can "vote" for. (So far, no one has voted for the text editing component. But there's still time to fill out the questionnaire.) If it comes out on top, it will be one of the next big things I will tackle.

f.view.lineOffset += diff
}
if f.cursor.y < y {
diff := -((lastIndexLine - indexLine) - lastCy) - 1
Copy link
Contributor

Choose a reason for hiding this comment

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

As best I can tell 'lastCy' shouldn't be involved in this calculation at all. When the textarea fills the screen lastCy will be zero when we hit the top of the screen and need to scroll earlier lines into view. When the textarea is offset from the top of the screen though, lastCy will be non-zero and this will cause diff to get ridiculous values. As a result lineOffset gets set to incorrect values and all your text disappears off the textarea display. Removing 'lastCy' from this calc made scrolling work as expected.

@rivo
Copy link
Owner

rivo commented Aug 26, 2023

TextArea exists now. (Since last year already.)

@rivo rivo closed this Aug 26, 2023
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

8 participants