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

Implement word selection for textfield, start word selection by double clicking and dragging #2580

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

ShaharMS
Copy link
Contributor

@ShaharMS ShaharMS commented Aug 8, 2022

Heres a PR that implements word selection.

To summarize, i made two external, non-completion variables - __wordSelection and __wordSelectionInitialIndex.

  • __wordSelection is used to check if the textfield is currently "double clicked". if so, regular dragging is modified to only account for word starts/endings as indices
  • __wordSelectionInitialIndex is used to know where to start the selection from, and to which direction (for example, if __caretIndex > __wordSelectionInitialIndex, we need to select from the start of the first selected word to the end of the last. othewise, select from the end of the first word selected, to the start of the last)

I also made two non-completeion functions to calculate the selection indices:

This one is used to set __caretIndex
https://github.com/ShaharMS/openfl/blob/f8d1f60c2391d6aaf0ebdb1be6e1a08b7aad443a/src/openfl/text/TextField.hx#L1964-L1987

And this one is used to set __selectionIndex
https://github.com/ShaharMS/openfl/blob/f8d1f60c2391d6aaf0ebdb1be6e1a08b7aad443a/src/openfl/text/TextField.hx#L1989-L2011

When first added and clicked, a timing error occurs when setting __caretIndex and __selectionIndex - `__selectionIndex` defaults to 0, and the double click verification cant change it in time, so the first click results in a selection from 0 to __caretIndex
@joshtynjala
Copy link
Member

regular dragging is modified to only account for word starts/endings as indices

Interesting! I knew that double-clicking a word would select the whole word, but I didn't know that if you dragged after double-clicking, it would expand selection to whole words instead of character by character.

@ShaharMS
Copy link
Contributor Author

ShaharMS commented Aug 9, 2022

another thing i didnt know (which i plan to implement too) is that a triple click selects an entire pragraph, and dragging selects from the initial paragraph to the one under the mouse. i thought it selected lines, but thats probably because i use vscode too much😆

@ShaharMS
Copy link
Contributor Author

ShaharMS commented Aug 9, 2022

Another thing, you might have noticed i removed the event listener for DOUBLE_CLICK. i still kept the function just in-case we want to revert some things, but the event listener can be removed

@ShaharMS
Copy link
Contributor Author

would this be merged right after 9.2? if so, should i also add the triple click, paragraph-selection capabilities?

@ShaharMS
Copy link
Contributor Author

I know what to fix to make CI working, I'll do the fix after 9.2 is out

@Dimensionscape
Copy link
Member

It just your use of the final keyword. This is only supported since haxe 4. Since we still support backwards compatibility with older versions, you should avoid using syntax only available in new releases of Haxe.

@ShaharMS
Copy link
Contributor Author

can someone trigger CI to check the last commit? i wan to see if there's anything i missed

@ShaharMS
Copy link
Contributor Author

ShaharMS commented Sep 1, 2022

Can this be merged now? i want to work on the other selection changes, but I'm wating for this to get merged

@ShaharMS
Copy link
Contributor Author

ShaharMS commented Dec 2, 2023

Bumping this, the selection changes, or at least further modification of the way openfl does text selection is really needed, especially for web platforms, on which OpenFl's textfields stick out like a sore thumb...
#2587 may still be relevant. If not, ill merge from develop and make progress on this PR

And another question - if this PR is still both relevant & applicable, can it be merged into 9.4.0-Dev instead? seems more fitting imo

@joshtynjala
Copy link
Member

I merged #2587 the other day, and I'd like to merge this one too (into 9.4.0-Dev). I think that this one should be updated to use clickCount, now that it is available (instead of the decideDoubleClick extra listener workaround), but I'm guessing that's probably the only change necessary.

@ShaharMS
Copy link
Contributor Author

I'm quite busy today, and there's also some work to do with paragraph selection iirc (triple clilck)
Will probably get to fix things tomorrow

ShaharMS and others added 4 commits April 11, 2024 11:01
…ubleClick event as it is not referenced anywhere and is not necessary
…by line (paragraph selection is the same thing, its name line selection for clarity
@ShaharMS
Copy link
Contributor Author

ShaharMS commented Apr 12, 2024

All is pretty much done! some things have changed since the start of this pr:

  • There are now two "special" selection modes: __wordSelection and __lineSelection. Double click selects text by word, while triple click selects text by line. Both support dragging, and maintain the selection mode throughout the drag
  • __wordSelectionInitialIndex has been renamed to __specialSelectionInitialIndex
  • __getPositionByWord and __getOppositeWordBound has been slightly changed to support line selection too, and are now called __getPositionByIdentifier and __getOppositeIdentifierBound.
  • onDoubleClick callback has been completely removed, as it has no use now - it was used for word selection only, and it's behavior wasn't consistent with most other text inputs/editors, as you had to release the mouse to select a word. It is now inside the MOUSE_DOWN, MOUSE_UP and MOUSE_MOVE callbacks

Things i haven't tested:

  • phones. They should behave the same in theory, though

Things i haven't implemented/done:

  • word selection using ctrl+shift+→ ←. works a little differently, and I didn't want to clutter this PR too much
  • fixing line selection ignoring the original line's length when dropping/rising to another, shorter line. Not too related to this PR, so i decided to do this in another PR if needed

@joshtynjala
Copy link
Member

@ShaharMS I may have some more feedback after I get into testing the code, but just reading through it, can you remove the doubleClickEnabled = true line in the constructor too? It appears to me that is no longer needed with the removal of this_onDoubleClick.

@ShaharMS
Copy link
Contributor Author

@ShaharMS I may have some more feedback after I get into testing the code, but just reading through it, can you remove the doubleClickEnabled = true line in the constructor too? It appears to me that is no longer needed with the removal of this_onDoubleClick.

Or maybe go all the way and only allow these types of selection when doubleClickEnabled is true? Which route should I go?

@joshtynjala
Copy link
Member

I checked the other day, and TextField's doubleClickEnabled is false in Flash. So just remove that line, and it's all good.

@ShaharMS
Copy link
Contributor Author

@joshtynjala line removed. Can this be merged?

@joshtynjala
Copy link
Member

Thanks! When I have time again, I'll do some testing, and merge it if everything looks good.

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