-
Notifications
You must be signed in to change notification settings - Fork 131
feat: rework path, remove suggestion prompt #335
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
Conversation
🦋 Changeset detectedLatest commit: e89cd5b 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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, we might need to update docs tho for this part, we can add them in the docs repo.
@manuel3108 would you be able to try this branch out in svelte? It includes the placeholder changes in main that unblock your unforking |
@dreyfus92 im going to open an issue to track updating the docs, as the original PR didn't add any either so we are missing a few entries |
After some thought recently, I came to the conclusion that the
path
andsuggestion
prompts shouldn't exist in their current form.The
path
prompt is a file selector - but is basically giving a poorer equivalent of the autocompletion UX. Instead of providing two ways to render completions, I think we should settle on the autocompletion prompt and remove the suggestion prompt.This means the
path
prompt now uses the autocompletion prompt, but withoptions
sourced from the file system.Notable changes
value
on<RETURN>
rather thanfinalize
event. This allows us to set thevalue
before validation occurs, so we can then validate against the completed pathbeforePrompt
and instead provides aninitialUserInput
which can be used to set the input without setting thevalue
validate
function and will render validation errorsBehavioural differences compared to the old
path
In the old
path
, you could enter a value which doesn't exist as an option and it would be set as the result on submit.In the new
path
, you must select a value. you can no longer submit arbitrary/unknown valuescc @dreyfus92
also @MacFJA you put plenty of work into these two prompts, i know. i think it is sensible to consolidate, though. we can maintain one core prompt instead of two here and reduce the maintainer burden. this will also keep the UX consistent.
let me know what you both think