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

SelectableLazyColumn perf/code review #242

Open
1 of 4 tasks
rock3r opened this issue Nov 3, 2023 · 2 comments
Open
1 of 4 tasks

SelectableLazyColumn perf/code review #242

rock3r opened this issue Nov 3, 2023 · 2 comments
Assignees
Labels
cleanup Improve the project/tooling and/or get rid of tech debt

Comments

@rock3r
Copy link
Collaborator

rock3r commented Nov 3, 2023

The SelectableLazyColumn is a foundational component that will be used everywhere. As such, we need to make it work as well as it possibly can. This issue is to track the need to do a thorough code and perf review of the component, fixing any issues we find.


Known concerns

  • allKeys: List<Key> and keyToIndex: Map<Int, Key> shouldn't exist at all, since the API related to it causes O(n) selection change operations, where they should be O(1)
  • SelectedKeys map can contain now-invalid keys (e.g., if the data changes or selected items are removed)
  • SelectableLazyColumn and Tree keybindings are broken #237
  • Deselecting items with the keyboard is not working in multi-select mode
@rock3r rock3r added the cleanup Improve the project/tooling and/or get rid of tech debt label Nov 3, 2023
@rock3r rock3r added this to the Milestone 10 — v0.10.0 milestone Nov 3, 2023
@rock3r rock3r self-assigned this Nov 3, 2023
@Walingar
Copy link
Collaborator

Walingar commented Nov 3, 2023

Also, there is a usecase that should be taken into account:

Imagine we have a textfield and lazy column where elements can be selected (e.g. Search Everywhere or Git Branches popup)
I want to propagate arrowDown/arrowUp presses to lazy list. So that when TextField is focused and arrowDown is pressed, lazy list become focused and arrowDown is handled by lazy list.

Now events handling is incapsulated in SelectableLazyColumn, so supporting this scenario is quite difficult

@rock3r rock3r modified the milestones: Milestone 13, Milestone 14 Jan 14, 2024
@rock3r rock3r assigned fscarponi and unassigned rock3r Jan 14, 2024
@rock3r rock3r modified the milestones: Milestone 14, Milestone 15 Feb 8, 2024
@rock3r rock3r modified the milestones: Milestone 15, Milestone 16 Feb 26, 2024
@rock3r rock3r modified the milestones: Milestone 16, Milestone 17 Mar 19, 2024
@fscarponi fscarponi removed their assignment Jul 9, 2024
@rock3r rock3r removed this from the Milestone 17 milestone Jul 9, 2024
@rock3r rock3r self-assigned this Aug 12, 2024
@rock3r
Copy link
Collaborator Author

rock3r commented Sep 30, 2024

Related: #470

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Improve the project/tooling and/or get rid of tech debt
Projects
None yet
Development

No branches or pull requests

3 participants