-
Notifications
You must be signed in to change notification settings - Fork 100
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
Fix Clear button always being visible in Playroom TextFields #1725
Conversation
|
43e4173
to
75912c7
Compare
75912c7
to
521a1a9
Compare
description: ( | ||
<Notice tone="info"> | ||
<Text> | ||
When prototyping in Playroom, fields will automatically include a{' '} | ||
<TextLink href="/components/IconClear">clear icon</TextLink> and | ||
clearing functionality. To disable this behaviour, set{' '} | ||
<Strong>onClear</Strong> to <Strong>false</Strong>. | ||
</Text> | ||
</Notice> | ||
), |
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.
Putting the docs messaging below the example to reduce prominence. Happy for suggestions otherwise
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.
Love it
Edit: switched solution to option 1
Problem
Currently when not setting onClear on a TextField the Clear button (Cross on the right side) isn't visible in applications and the documentation site.
Example: Documentation Site - No Clear Button
However, it is visible in Playroom, due to Playroom's automatic injection of state handling.
Example: Playroom - Clear Button
This creates a discrepancy between what users see in docs examples and playrooms, and makes it impossible create playrooms where the Clear button is not visible.
Solution
Avoid passing onClear through to the TextField in Playroom when it's falsy.
To make it easy for prototyping to add the clear field behaviour allow passing
true
instead of a no-op function.Example: Playroom - With Change now has no Clear button
When onClear is not provided:
There would now be no clear button:
Playroom
Alternative Options
The downside with the above solution is that playrooms now use invalid code
onClear={true}
in prototypes that can't be copied into application code.Alternative 1 - Opt-out of Clear label: We could default to showing the Clear button (as we do currently) but allow passing
onClear={false}
to deliberately opt-out of the clear label.This makes the minimal change, doesn't effect existing playrooms but doesn't fix the disconnect between examples and playrooms.
Alternative 2 - Authentic behaviour: We could simply not allow passing a boolean in, requiring users to enter a dummy function
onClear={() => {}}
to activate the clear button.This may be confusing to playroom users who aren't familiar with JavaScript as well as breaking existing playrooms.