feat(core + prompts): adds autocomplete#288
Conversation
🦋 Changeset detectedLatest commit: 984a92d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@example/basic • @example/changesets commit: |
MacFJA
left a comment
MacFJA
left a comment
There was a problem hiding this comment.
IMO the hint always visible feel a bit overwhelming, that add a lot of gray (and informative) text
There is also some consistency issue
But I really like the presentation:
- informative line above and under the content
- the content being indented to separate it from the informative part
natemoo-re
left a comment
natemoo-re
left a comment
There was a problem hiding this comment.
Looks really awesome! A few minor points on the visual feedback, also noticing a bug with an empty line at the top and some issues switching from multiselect to filtering.
dreyfus92
changed the title
MacFJA
left a comment
MacFJA
left a comment
There was a problem hiding this comment.
I think the best option to handle switching between navigation and filtering mode is to expose the _track property of the Prompt class.
In navigation _track is false, and true when filtering.
The value event is only triggered when _track is true,
so if _track is false we don't have to worry about space being added in the value.
But in exchange, we have to readd the key that change the mode from navigation to filtering
|
This PR will replace #124 |
| const { cursor, options, style } = params; | ||
| const output: Writable = params.output ?? process.stdout; | ||
| const rows = output instanceof WriteStream && output.rows !== undefined ? output.rows : 10; | ||
| const overflowFormat = params.overflowFormat ?? color.dim(' ...'); |
There was a problem hiding this comment.
did we ever use this? it looks like the only places we set it, we use the default
There was a problem hiding this comment.
i'm not sure about this one tbh.
There was a problem hiding this comment.
looking at the snapshots, it seems this changes how some of the existing prompts render
so maybe we should just revert this bit as it seems unnecessary for the autocomplete
There was a problem hiding this comment.
should we maybe revert this @dreyfus92?
unless you can figure out why it exists
There was a problem hiding this comment.
I will try to dive into this tmr 🫡, this might be the last piece right?
There was a problem hiding this comment.
@43081j, I've noticed that overflowFormat param is used in 2 places: select.ts and multiselect.ts, the key difference is that the default has 4 spaces while the explicit usages have 2 spaces. This is likely what you noticed in the snapshots, the rendering would be different between the def and explicit values. The param is used to show an elipsis when there are more options than can be displayed in the terminal window, indicating that there are more items above or below the visible options.
I would suggest to keep the param but standardize the spacing. The explicit usages in both select.ts and multi-select.ts use 2 spaces which seems to be the intended design. We could update the default in limit-options.ts to match this standard of 2 spaces instead of 4. This way, we maintain the flexibility of the param (in case someone wants to customize it) while having consistent behavior.
Let me know if you want me to move forward with this recommendation 😁
There was a problem hiding this comment.
the multi-select prompt had no spaces before, and now has two, because of this
that's what i want to understand. does it look better with the two spaces? why were they added?
|
looks good to me! feel free to merge |
|
Feature wise this works awesome! Unfortunately, I am seeing this weird glitch where things appear multiple times. Using both Ghostty and built-in Mac terminal. |
|
hey @grabbou can you open an issue please? 😁 |
5 participants
Related: #203 💣