-
Notifications
You must be signed in to change notification settings - Fork 153
feat: add withBorder option #409
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 2d0be2f 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: |
dreyfus92
left a comment
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 😁
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.
I'm definitely in favor of anything that makes theming easier! Always hoped people would use @clack/core to create their own branded experiences, but that package is still a bit too low-level...
Maybe we should consider adding a theme to the global ClackSettings that could be a hook for global stylistic changes like this? Seems a bit frustrating to require passing withBorder: false every time.
|
the global settings have their downsides too, since you may not always want to every prompt to follow these rules maybe in a follow up change, we can make the global settings inherit some of the shared settings, and the inline option takes precedence. i.e. if you set global |
delucis
left a comment
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.
Finally got a minute to take a look! Thanks @43081j!
|
i've renamed this to @delucis could you take another look? |
This adds a new
withBorderoption which decides if the standard clack left border is rendered or not.Not all prompts implement this yet. We can add support for more prompts over time.