Skip to content

fix: treat placeholders as visual element #323

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

Merged
merged 4 commits into from
May 26, 2025
Merged

Conversation

43081j
Copy link
Collaborator

@43081j 43081j commented May 18, 2025

This changes how we deal with placeholders.

Currently, a placeholder behaves as a visual hint of what we want the user to type. It also behaves as an optional default value, in that you can press <Enter> or <Tab> to insert it as the value.

This is confusing since it means it is a "partial" default value, and a placeholder at the same time.

This change basically removes the placeholder logic from core, such that the text and autocomplete prompts use placeholder purely as a render/visual hint.

This updated behaviour means we render the placeholder when no value is set, but it can no longer be inserted as a value. To achieve that, you should combine defaultValue with placeholder (i.e. set them to the same value).

cc @dreyfus92 @natemoo-re

Copy link

changeset-bot bot commented May 18, 2025

🦋 Changeset detected

Latest commit: 8e567c9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@clack/prompts Patch
@clack/core Patch

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

Copy link

pkg-pr-new bot commented May 18, 2025

@example/basic@example/changesets

npm i https://pkg.pr.new/bombshell-dev/clack/@clack/core@323
npm i https://pkg.pr.new/bombshell-dev/clack/@clack/prompts@323

commit: 8e567c9

43081j added 2 commits May 18, 2025 21:11
This changes how we deal with placeholders.

Currently, a placeholder behaves as a visual hint of what we want the
user to type. It also behaves as an optional default value, in that you
can press `<Enter>` or `<Tab>` to insert it as the value.

This is confusing since it means it is a "partial" default value, and a
placeholder at the same time.

This change basically removes the placeholder logic from core, such that
the `text` and `autocomplete` prompts use `placeholder` purely as a
render/visual hint.

This updated behaviour means we render the placeholder when no value is
set, but it can no longer be inserted as a value. To achieve that, you
should combine `defaultValue` with `placeholder` (i.e. set them to the
same value).
@43081j 43081j force-pushed the placeholder-fixeroos branch from c52b13d to 802a24e Compare May 18, 2025 20:12
Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea, but i don't know if it aligns with nate 😁

@43081j
Copy link
Collaborator Author

43081j commented May 18, 2025

the original idea seemed to be that placeholder was a value you can tab in, like an optional default

but we can achieve this with defaultValue a bit more cleanly i think

if you set defaultValue in a text prompt, pressing <enter> will set the value as that. meanwhile, the placeholder is just to show a message until the user types something in

i think this makes more sense than whats in main

Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that the changeset wasn't detected by the bot 😅

@manuel3108
Copy link

This aligns with what I'm expecting and was used to in 0.x. Additionally, this seem like the cleaner way for me. Strongly approve.

@43081j 43081j merged commit 94fee2a into main May 26, 2025
6 checks passed
@43081j 43081j deleted the placeholder-fixeroos branch May 26, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants