JetBrains / jewel

An implementation of the IntelliJ look and feels in Compose for Desktop
Apache License 2.0
636 stars 30 forks source link

Tabbing into a selectableLazyColumn with no selections pre-made does not allow keyboard navigation #357

Closed YasserDbeis closed 2 months ago

YasserDbeis commented 2 months ago

If the user has not selected anything from a SelectableLazyColumn, and navigates to the SelectableLazyColumn using tab, then the arrow keys do not seem to be detected. I was able to reproduce this issue by using the ide-sample's Compose Panel tab's list of releases as the SelectableLazyColumn.

Note: this has been encountered with https://github.com/JetBrains/jewel/issues/347 merged.

fscarponi commented 2 months ago

This behavior is a known issue in the swing feature pair. You can verify it in the IDE: when a "LazyColumnLike" component (such as the project tree) gains focus, the first item is selected. This allows you to navigate freely using arrow keys. The same applies to SLC if you set a selection either through clicking or programmatically.

Why is the scenario of "selecting the first element" not desirable? Unlike Swing, the lazy column is not fully loaded into memory, so it's possible that an element with key k is selected even if it's not currently in the visible range... In that case? should the list scroll or what? Other possible scenarios come to mind that would need to be addressed...

However, nothing prevents the user from setting the selected element by manipulating the state during the construction of the list! It would also be possible to add the key of the first element to the set of selected items if it's empty, but this would result in losing support for "nothing selected."

Feel free to suggest any other solution!

rock3r commented 2 months ago

I think Swing selects the first visible item, or the last selected item, when a list gains focus. We should do the same thing.

If the last selected item key doesn't exist anymore, I'd select the first visible one. I think that's what Swing does (but will double check). I agree with Yasser that focusing the component should allow you to keyboard navigate immediately

rock3r commented 2 months ago

Screen recording of Swing list behaviour

So, I checked and the Swing behaviour is as follows:

  1. When focusing a list, either the previous selection is kept, or the first item gets selected (if there is any item in the list)
  2. When the current selection is gone (e.g., item removed), the item above (i.e., selectedIndex - 1) is selected
  3. If the item was the first one, then the next one is selected (i.e., the selection remains on selectedIndex even if the item is now different)
  4. If there are no items left, the selection is null
  5. The selection can only be null when the list is empty, or if it has never received focus before

On multi-select lists, it pretty much follows the same rules. The only difference is that when there are multiple items selected and one of them disappears, the item also disappears from the selection, but other selected items remain selected. If all selected items are removed, the behaviour is the same as for the single-select list.

We should align to this behaviour. If the list is empty when it receives focus, it'll have no selection, but as soon as the user uses the keyboard to move the selection, a selection is created on the first item. If apps want to create a selection once they add data to the list, they're free to do so, but we won't do it for them.

YasserDbeis commented 2 months ago

I agree with the proposed solution. Thanks for the investigation!

rock3r commented 2 months ago

@fscarponi please look into this when you have time 🙏

fscarponi commented 2 months ago

Hi Folks, there is not too much effort to get the desired result, except for point 2... The current implementation of the data structure is not minded to store ordered keys, so I'm not sure if it is possible to achieve this behavior without rethinking the key management, we need to investigate deeper. (keep in mind that the SLC supports selectable AND not-selectable elements, such as the headers) The problem is that I'm busy at the moment and can't provide an ETA. If you need this asap, I can try to discuss/schedule this task with my TL

YasserDbeis commented 2 months ago

@fscarponi The only thing on my end needed ASAP is the ability to tab into the selectableLazyColumn and use arrow keys when there is no selection made before (the original problem). I say this because the Profiler needs to support full keybaord navigation; one of the major components on opening the UI is a selectableLazyColumn, and the user cannot make their selection if they are purely using the keyboard. I am okay with the other behavior coming later if that is alright.

fscarponi commented 2 months ago

372 add an hotfix for the point 1

When focusing a list, either the previous selection is kept, or the first item gets selected (if there is any item in the list)

to implement other behaviors is possible but requires more investigations, keeping this state easily savable and consistent. I will take care to create a new issue for these features