Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Dyamic max results #98

Closed
wants to merge 5 commits into from
Closed

Dyamic max results #98

wants to merge 5 commits into from

Conversation

t9md
Copy link
Contributor

@t9md t9md commented Nov 16, 2017

Description of the Change

@jarle , @nathansobo How you think this?

This is proof of concept to solve #80.

Approach taken in this PR is different from #94
So changes done in #94 is intentionally reverted to re-think problem clearly again.

  • This PR is not finished, Test is broken, just wanted to show different idea to tackle initial-open lag issue.
  • I thinks this "dynamic truncation/un-truncation to reduce items to attache to DOM" feature can be implemented in atom-select-list rather than command-palette.
  • But works as expected in UX-wise(faster initial open response time).
  • console.log is not removed so that other people can easily correlate following GIF to code.

command-palette-package_js_ ___github_command-palette

Alternate Designs

#94

Benefits

Stupidly simple.

Possible Drawbacks

When user mouseover, first core:move-down(down arrow key), it cause full item rendering.
So user experience lag at this timing.
But this is acceptable drawbacks since experience lag occasionally is far better than experience it always.

Applicable Issues

#80.

@nathansobo
Copy link
Contributor

Still seems like the ideal approach would be to only render items as they become visible, but if this solves our immediate issue then if someone can get it green, I say we can proceed with it.

@t9md
Copy link
Contributor Author

t9md commented Nov 18, 2017

Thanks, I will do.
Also try with better implementation.

Provides dynamic rendering when it become visible by atom-select-list is best solution.
Maybe enable it by passing dynamicRendering option to avoid breaking specs for other select-list using pkg..
Will see...

@t9md
Copy link
Contributor Author

t9md commented Nov 27, 2017

Close in favor of #101

@t9md t9md closed this Nov 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants