feat: add withBorder option #409
Conversation
🦋 Changeset detectedLatest commit: f8bf95a 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.
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 |
| const contentPadding = opts?.contentPadding ?? 2; | ||
| const width = opts?.width === undefined || opts.width === 'auto' ? 1 : Math.min(1, opts.width); | ||
| const linePrefix = opts?.includePrefix ? `${S_BAR} ` : ''; | ||
| const linePrefix = opts?.withBorder === false ? '' : `${S_BAR} `; |
There was a problem hiding this comment.
Does this means boxes would render without a left border? i.e. something like
╭───────────────────────╮
example box │
some more text... │
╰───────────────────────╯
I’m not sure, but this one feels like even when rendering without a left border, you’d still want boxes to render with the four sides intact. At least if this is a generic option set at a global level. (And if a local thing like box('example', null, { withBorder: false }), then I’d expect no border at all, not only the left border to be missing.
Maybe there’s an argument that what we’re describing as a “border” is not really a border in the generic rendering sense. It has a UI function in connecting prompts, guiding the eye from step to step, etc. So something like withGuide could be a more appropriate naming? And then that would make it more obvious that a box would not be impacted (but might have other styling hooks given it genuinely is a bordered element).
There was a problem hiding this comment.
That's a fair point
This is configuring the "guide" , the box would still have a left border (as part of the box).
withGuide might work 🤔 but yeah that's basically what I'm trying to make configurable here - the left "guide line" 😅
| input?: Readable; | ||
| output?: Writable; | ||
| signal?: AbortSignal; | ||
| withBorder?: boolean; |
There was a problem hiding this comment.
See above question re: whether this is the correct naming or if it should be withGuide or similar.
| render() { | ||
| const title = `${color.gray(S_BAR)}\n${symbol(this.state)} ${opts.message}\n`; | ||
| const withBorder = opts.withBorder !== false; | ||
| const titlePrefix = withBorder ? `${color.gray(S_BAR)}\n${symbol(this.state)} ` : ''; |
There was a problem hiding this comment.
IIUC this means no symbols prefixing prompts when disabling the border? It’s personal preference I guess, but I’d still expect some kind of leading marker even when disabling borders/guides. These markers can be helpful indicators of state.
For example, to mirror something a bit like what prompts does, state is indicated via something like
? What is your name?
Placeholder
And then post-confirm
✔︎ What is your name?
Chris
These could definitely just be the standard clack symbols by default but the left border/guide seems a distinct UI element from these status indicators to me.
There was a problem hiding this comment.
It looks a little weird without some rework since the symbol shows up where the guide line would usually be, detached from the box
I'll have a think
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.