add Popover docs #1598
add Popover docs #1598
Conversation
mayank99
requested review from
gretanausedaite and
siddhantrawal
and removed request for
a team
| onVisibleChange={setIsOpen} | ||
| style={{ maxWidth: '45ch', border: 'none' }} | ||
| content={ | ||
| <Surface elevation={0}> |
There was a problem hiding this comment.
I had to add border: none override because Surface always insists on bringing its own border.
Also, for some reason Surface.Body only brings horizontal padding not vertical.
@FlyersPh9 Is this intentional?
There was a problem hiding this comment.
data-iui-padded="true" does have vertical padding, it's just very little.
The examples on the surface demo page look nice, but the agree button in your screenshot does not look so great. Was the padding changed when we implemented scrollbar-gutter?
Btw, just now found out scrollbar-gutter doesn't work in Safari. 😓
examples/Popover.focus.tsx
Outdated
| </Text> | ||
|
|
||
| <Button | ||
| ref={(el) => el?.focus()} |
There was a problem hiding this comment.
nit: It might be helpful if there were an explanation comment here. Something like, "To grab focus when the popover opens".
examples/Popover.focus.tsx
Outdated
| <Surface.Body isPadded> | ||
| <Flex flexDirection='column' alignItems='start'> | ||
| <Text as='p' id={descriptionId}> | ||
| (TODO: improve text) Lorem ipsum dolor sit amet consectetur |
There was a problem hiding this comment.
@gretanausedaite @rohan-kadkol @siddhantrawal Can somebody suggest better text or a different example? I dunno how this has 3 approvals with TODO text 😅
There was a problem hiding this comment.
Can somebody suggest better text or a different example?
How about a "see image preview" popover? If we click on the trigger button that says "show image preview", the popover shows the image preview along with an "OK" button and a "Download" button.
I dunno how this has 3 approvals with TODO text 😅
I thought that TODO was for you so you will handle it yourself 😅 So I gave a preemptive approval to not block the PR because everything else LGTM.
There was a problem hiding this comment.
@gretanausedaite @rohan-kadkol @siddhantrawal Can somebody suggest better text or a different example? I dunno how this has 3 approvals with TODO text 😅
Maybe you can add something related to Bentley agreement or terms of use. If not, short summary about what you can add in the popover.
There was a problem hiding this comment.
Can somebody suggest better text or a different example?
Some sliders to "tune settings", eg. quality, grain, color, saturation, etc. (like in photoshop) with ok or save / cancel buttons at the bottom.
There was a problem hiding this comment.
I dunno how this has 3 approvals with TODO text
We have more than one todo in repo that is supposed to be solved later.
There was a problem hiding this comment.
updated the example to show a settings dialog.
these are just text inputs for now, but let me know if you have ideas for other kinds of inputs
There was a problem hiding this comment.
It's good. We have this in css fieldset tests if you want to switch inputs but you don't have to. This example is to show focus control, it does not have to be useful or realistic.
|
React Visual Tests related to one of the Color Picker component is failing. |
Changes
Changes to docs: added new
/popoverpage with usage and accessibility details.While working on docs, I found I had to make changes to Popover component (now moved into #1602).
Testing
Tested all examples in multiple browsers and with keyboard.
Docs
n/a. These are docs.