-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat(Forms): add inline HelpButton to all Field.* components as default (with option to open in Dialog) #4280
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
91feb9e
to
acaa0d9
Compare
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
acaa0d9
to
9425f77
Compare
9425f77
to
314c75e
Compare
b50b7b8
to
9ea92e4
Compare
9ea92e4
to
989ce7d
Compare
trim.CD1E5E30-B40D-4CBF-8836-6DD8B22274B4.MOVWhen closing, the title/header of the message jumps/shifts upwards a bit. Not very important, but it's noticeable. |
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.
Looks very good.
I've not read it all yet.
- Have we added some docs/changelog presenting that the default behavior of the help text/property is now inline(previously a dialog).
- And if some users still want to use the dialog variant, we could add what they need to do.
- some docs about when we suggest using the dialog variant. You mentioned something in the lines of long texts, etc.
- Is it only strings that can be provided to the help property? Or React.node's as well?
Also, when pressing the help button the input Field gets focus/active state/styling. Is that intentional and desired? Not sure myself. |
989ce7d
to
1e06e8f
Compare
This is on purpose. But badly executed. Added a 40ms delay, and now I think it fells perfect. |
This is a side effect. It's the "hover effect". First I have tried to have the HelpButton outside the label, but that creates other/larger layout/placement issues. I fear we have no choice. But I will take another look if there is way to avoid that. |
Not yet. I think we should add that in #4279
<Field.String
help={{
title: 'Title',
content: 'Content.',
+ renderAs: 'dialog'
}}
/>
React.Node |
I'm not sure what happened here 🤷 and because it can't be repoened (why?) I create a new one #4282 |
This PR changes the existing behavior of the
help
prop for Fields that previously supported it. The content now opens inline instead of in a Dialog.The
help
prop has now all of these optional props:Examples with inline "HelpButton":
NB: We do not document the internals of the inline version at this time. This can be included in another PR. The reason is that it would require additional tests, examples, and documentation. It is also an "isolated" feature specific to the HelpButton. However, as of this writing, I am unsure if we will ever document it, as integration is not straightforward. There are several considerations to address when implementing it.