Add arrows selection to typeahead #3386
Conversation
| @@ -0,0 +1,75 @@ | |||
| const scroll = (size, { offset, max }) => { | |||
There was a problem hiding this comment.
Needs to be moved and cleaned
|
Yes! It looks awesome |
Generated by 🚫 dangerJS |
rogeliog
force-pushed
the
add-arrows-to-typeahead
branch
from
df8db39 to
f3a7f40
Compare
| .map(name => formatTestNameByPattern(name, pattern, width - 4)) | ||
| .map((item, i) => { | ||
| if (i === index) { | ||
| this._prompt.setTypheadheadSelection(chalk.stripColor(item)); |
There was a problem hiding this comment.
Should this be this._prompt.setTypheadheadSelection('^' + chalk.stripColor(item) + '$'); instead?
rogeliog
changed the title
thymikee
force-pushed
the
add-arrows-to-typeahead
branch
from
f3a7f40 to
b253111
Compare
| updateCachedTestResults(testResults: Array<TestResult>) { | ||
| this._cachedTestResults = testResults || []; | ||
| updateCachedTestResults(testResults: Array<TestResult> = []) { | ||
| this._cachedTestResults = testResults; |
There was a problem hiding this comment.
Any particular reason for this change?
There was a problem hiding this comment.
I just like initial values better :)
| // eslint-disable-next-line max-len | ||
| `\n\n ${chalk.italic.yellow('Start typing to filter by a filename regex pattern.')}`, | ||
| ); | ||
| printStartTyping('filename', pipe); |
There was a problem hiding this comment.
this could use this._entityName, right?
There was a problem hiding this comment.
Actually, nevermind, it's more of a coincidence that they line up. Let's keep it this way.
There was a problem hiding this comment.
I see now that I'm mixing singular with plural in couple of places, need to fix it properly.
|
This is really fantastic. There is one bug that we found about the arrow selection scrambling the results but other than that it looks really solid. There are a few more things we could do, like using composition instead of inheritance and some stricter flow types (ie. not using Function) but given that we are pressed on time here, I would suggest merging it after fixing the one bug and then, if we have time, improving it further once @rogeliog is back. @thymikee and @rogeliog both, this is awesome. |
Codecov Report
@@ Coverage Diff @@
## master #3386 +/- ##
==========================================
+ Coverage 64.1% 64.38% +0.28%
==========================================
Files 179 182 +3
Lines 6647 6694 +47
Branches 5 5
==========================================
+ Hits 4261 4310 +49
+ Misses 2384 2382 -2
Partials 2 2
Continue to review full report at Codecov.
|
* Add typeahead arrow selection * Handle max typeahead max offset * Add tests for scroll list * Add simple tests to typeheahead selection * Clear typeahead selection on enter * Fix prompt test * Refactor pattern prompts * Stress 'cached' in TestNamePatternPrompt * Fix eslint * Fix scrolling edgecase * Live update number of remaining typeahead items
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fixes #2589
This puts a lot of the typeahead logic inside the prompt class, but we should move it to a typeahead module eventually.
Still needs to be tested in Windows and other terminals
We still need to do all of these as part of a follow up PR
Summary
Also, do we even want to move forward with this?