lime icon indicating copy to clipboard operation
lime copied to clipboard

focus back to textInput after setting clipBoard

Open junsred opened this issue 2 years ago • 8 comments

junsred avatar Apr 28 '22 12:04 junsred

@jgranick please check this pr

junsred avatar Apr 28 '22 12:04 junsred

What is the importance of the Timer here?

Dimensionscape avatar Apr 28 '22 14:04 Dimensionscape

What is the importance of the Timer here? @Dimensionscape

without timer it doesn't copy + timer makes sure game doesn't stop when player spams paste

junsred avatar Apr 28 '22 14:04 junsred

Looks like we were already doing this:

private function handleFocusEvent(event:FocusEvent):Void
{
	if (textInputEnabled)
	{
		if (event.relatedTarget == null || isDescendent(cast event.relatedTarget))
		{
			Timer.delay(function()
			{
				if (textInputEnabled) textInput.focus();
			}, 20);
		}
	}
}

So I can't complain too much.

Actually wait, how's this for an idea? Instead of copy-pasting a questionable block of code, call handleFocusEvent(). That way, if we ever come up with a better solution, we only need to update the handleFocusEvent function.

if (Browser.document.queryCommandEnabled("copy"))
{
	Browser.document.execCommand("copy");
}
-Timer.delay(function()
-{
-	if (textInputEnabled) textInput.focus();
-}, 20);
+handleFocusEvent(new FocusEvent("focus"));

Maybe also rename handleFocusEvent to restoreFocus or ensureFocus or something? Some name that makes it clearer why you'd call the function. This part's optional, though.

player-03 avatar Apr 28 '22 17:04 player-03

@player-03 how about this commit?

junsred avatar Apr 28 '22 21:04 junsred

Without this code, copying selected text in textInput causes textInput to lose focus and client loses ability to input.

junsred avatar Apr 28 '22 21:04 junsred

I wonder if the function name could be shorter.

  • refocusTextInput
  • restoreFocusAfterDelay
  • restoreInputFocus

But there's always a trade-off. Longer names are explicit, shorter names are easier to remember. Personally, I err on the side of shorter names, adding a comment if I need to explain further.

For example:

var __focusPending = false;

/**
	Restores focus to `textInput`, making sure not to change it too many times in succession. Changing focus multiple times
	per frame can sometimes interfere with copying or pasting text, as browsers may ignore all but the final focus target.
**/
public function focusTextInput():Void
{
	if (__focusPending) return;
	__focusPending = true;

	Timer.delay(function()
	{
		__focusPending = false;
		if (textInputEnabled) textInput.focus();
	}, 20);
}

The comment doesn't have to mention the delay, it just has to explain why the delay exists. (Same with __focusPending.) A good comment explains the problem being solved, so if someone finds a better solution, the explanation still applies.

player-03 avatar Apr 28 '22 21:04 player-03

@joshtynjala could you check this pr? Right now client is unable to input after copying text. This pr solves the problem.

junsred avatar Apr 30 '22 15:04 junsred