feat: add accessible flag and environment variable for screen reader friendly forms#455
feat: add accessible flag and environment variable for screen reader friendly forms#455
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #455 +/- ##
==========================================
- Coverage 71.23% 71.22% -0.02%
==========================================
Files 222 222
Lines 18664 18678 +14
==========================================
+ Hits 13295 13303 +8
- Misses 4189 4192 +3
- Partials 1180 1183 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zimeg
left a comment
There was a problem hiding this comment.
@srtaalej So amazing to find this feature arriving here! 🍀 ✨
Before approving I want to leave a few comments, but think we can follow up with some other improvements to the edges:
- Environment Variable: The
ACCESSIBLEenvironment variable is standard I understand and we should include support alongside these changes! This makes preferring these prompts easier with anexportadded to the shell profile 🔬 - Blank Selections: The "select" prompts request a number to be entered but accept blank input for the first option! This might be an upstream issue 🐛
- Input Default: Adjacent for the "input" prompt I find the default value of the
createcommand name isn't showing 🧪
The first note is most important for these changes IMHO! I'd be curious if the latter two can be proven in unit tests, but please don't consider those blocking 🙏
| SlackTestTraceFlag bool | ||
| TeamFlag string | ||
| TokenFlag string | ||
| Accessible bool |
There was a problem hiding this comment.
🌲 thought: As we're introducing this, should we include the environment variable similar?
ACCESSIBLE
We recommend setting this through an environment variable or configuration option to allow the user to control accessibility.
🔗 https://github.com/charmbracelet/huh?tab=readme-ov-file#accessibility
There was a problem hiding this comment.
🔍 ramble: We might find this adjacent file most useful!
slack-cli/internal/config/dotenv.go
Lines 23 to 24 in f39d175
Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>
zimeg
left a comment
There was a problem hiding this comment.
@srtaalej Thanks for these updates! I'm hoping we can land a few more to bring this to a polished state for release 🔬 ✨
I tested some more cases that I'll share here:
- One selection option: The hint is solid IMHO! I share more comments on the title format later but I'm a fan of the hint.
- Multiple select options: This highlights the ":" format a bit more for following comments.
- Invalid select options: Super cool that this validates!
- Confirmation default: This appears as a kind callout.
- Input prompts without default: These might favor the trailing ":" also?
The formatting issue I callout in comments below and some screenshots above is that I think form titles should end with a ":" to signal input! Some workarounds might be needed in code but this outputs well I think.
Let me know if more testing can be requested 🫡
| func buildInputForm(io *IOStreams, message string, cfg InputPromptConfig, input *string) *huh.Form { | ||
| title := message | ||
| if io != nil && io.config.Accessible && cfg.Placeholder != "" { | ||
| title = fmt.Sprintf("%s (default: %s)", message, cfg.Placeholder) |
There was a problem hiding this comment.
| title = fmt.Sprintf("%s (default: %s)", message, cfg.Placeholder) | |
| title = fmt.Sprintf("%s (default: %s):", strings.TrimSuffix(message, ":"), cfg.Placeholder) |
🔭 suggestion: This might not be the right syntax but I think formatting input ends best with a colon:
- Name your app: (default: flamboyant-salamander-784)
+ Name your app (default: flamboyant-salamander-784):There was a problem hiding this comment.
🎨 ramble: We don't have a comment on this in the STYLE_GUIDE at this time and we're not consistent on using a : at all or sometimes ? instead. This might be something nice to align over time.
|
|
||
| title := msg | ||
| if io != nil && io.config.Accessible && len(options) > 0 { | ||
| title = fmt.Sprintf("%s (press Enter for 1)", msg) |
There was a problem hiding this comment.
| title = fmt.Sprintf("%s (press Enter for 1)", msg) | |
| title = fmt.Sprintf("%s (press Enter for 1):", strings.TrimSuffix(msg, ":")) |
🪬 suggestion: Similar to the : suggestion above - I'm more hesitant to this one because some titles might end with "?" but I don't think that formatting is incorrect...
Changelog
Interactive forms become screen reader friendly when the
ACCESSIBLEenvironment variable is set or when using the--accessibleflag.Summary
Related: #454
This PR adds a global --accessible flag that switches huh interactive prompts to accessible mode. In accessible mode, select prompts render as numbered lists with "Enter a number between 1 and N" input, confirm prompts accept y/n text, and input prompts use plain line-by-line I/O. This makes the CLI usable with screen readers by avoiding the TUI-based rendering
Test plan
Run
slack create --accessibleand verify prompts render as numbered listsRun
slack login --accessibleand verify confirm prompt accepts y/nRun
slack create(without flag) and verify normal TUI prompts still workgo test ./internal/iostreams/ -run TestFormsAccessibleRequirements