Monday, February 10, 2025

More bugs in LeafComponentInput.focus()

I stumbled into #5919 (Cannot change the ticket number of a session in WorkedHours). This is obviously yet another side effect of #5893 (The grid cell editor should start by selecting all its content).

We can see it on Jane. Click on the Ticket cell of some existing session. More precisely it starts editing, correctly selects the whole content of the field, but then it never lets me enter more than one character because after every character it selects the whole thing again. This is the same problem as I saw for password fields.

I had a look at the two changes.

  • 202505202 : Fixed #5893 (The grid cell editor should start by selecting all its content) with commit 3f6d577d (“optimize for: #5893 (The grid cell editor should start by selecting all its content)”)

  • 20250207 : Fixed #5915 (Cannot manually enter a password) with commit 4c7a1024 (“allow input.select only in grid cell(s)”)

Here is the LeafComponentInput.focus() method before and after these changes.

Before:

focus() {
  let ref = this.inputEl;
  if (ref.focus) ref.focus()
  else if (ref.focusInput) ref.focusInput.focus()
  else if (ref.inputRef) ref.inputRef.current.focus()
}

First change (Fixed #5893 (The grid cell editor should start by selecting all its content)):

focus() {
  let ref = this.inputEl, input;
  if (ref.focusInput) ref = ref.focusInput
  else if (ref.inputRef) ref = ref.inputRef.current;
  if (ref.focus) {
      ref.focus()
      if (ref.hasOwnProperty('select')) {ref.select(); return}
      if (this.container) input = this.container.getElementsByTagName('input')[0];
      if (input) input.select();
}

Second change (Fixed #5915 (Cannot manually enter a password)):

focus() {
  let ref = this.inputEl, input;
  if (ref.focusInput) ref = ref.focusInput
  else if (ref.inputRef) ref = ref.inputRef.current;
  if (ref.focus) {
      ref.focus()
      if (this.props[constants.URL_PARAM_WINDOW_TYPE] === constants.WINDOW_TYPE_TABLE) {
          if (ref.hasOwnProperty('select')) {ref.select(); return}
          if (this.container) input = this.container.getElementsByTagName('input')[0];
          if (input) input.select();
      }
  }
}

LeafComponentInput is the base class for all input fields. It is defined in LinoComponentUtils.jsx where it inherits from LeafComponentBase and has itself a subclass LeafComponentInputChoices. It is subclassed by all input fields, including AutoComplete, DTFieldElement, TextFieldElement, ChoiceListFieldElement, ChoicesFieldElement, etc.

OMG! What are the meanings of the attributes inputEl, focusInput and inputRef? I started to read How to set focus on an input field after rendering in React. Trying to understand these things would require me several man days! I am glad to have Sharif!

But if you ask me, the second change, making it depend on the window type, is bullshit. I think you can safely undo this change. It just disables the new behaviour when we aren’t in a grid. This condition is unrelated. We want Lino to select the content of a field, including password fields, each time the field gets focus, also in an action dialog, also in a detail form. One exception: we do not want this in a TextField field.

The problem after the first change was that it selects the whole thing again after each keyboard stroke. It seems that the focus event is called again when selection changes.

But here I stop. Sharif, I hope that my thoughts helped you to continue.

P.S. : Sharifs answer is here: Commit f979de77.

About plugin dependency

For #5916 I had to (wanted to) put a text “Screenshot : [file 12]” into the description of some tickets.

These demo tickets get generated by the demo fixture of the noi plugin.

Adding references to screenshots in the description of tickets requires these tickets to exist already.

The demo screenshots get created in the demo fixtures of uploads.

So I said that the noi demo should load after the uploads demo.

For this I changed the lino_noi.lib.tickets and lino_noi.lib.cal plugins to no longer declare lino_noi.lib.noi as needed.

I commented out the needs_plugins = [‘lino_noi.lib.noi’] in lino_noi.lib.cal:

from lino_xl.lib.cal import Plugin
class Pugin(Plugin)
    # needs_plugins = ['lino_noi.lib.noi']
    ...

This had the unwanted side effect of reactivating the needs_plugins of lino_xl.lib.cal (but I understood this only later).

I think that my fundamental mistake was to increase the complexity of the noi demo fixture.

So I now load the screenshots in a new demo2 fixture of noi.

En passant I changed the xl cal plugin to no longer need checkdata and linod.