Enable Popovers with a header, and inside the header is the title and close button.
- Add a header slot.
- Add props for the title, close button, and close button label.
Description
Description
Details
Details
Related Changes in Gerrit:
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T386487 [EPIC] Support Trust & Safety Product with the UserReputationCard experiment | |||
| Resolved | DTorsani-WMF | T363375 [EPIC] Popover: Add Popover component to Codex | |||
| Resolved | AnneT | T386150 Popover: support header and header options |
Event Timeline
Comment Actions
Change #1121459 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):
[design/codex@main] Popover: Add header and footer content
Comment Actions
Change #1121459 merged by jenkins-bot:
[design/codex@main] Popover: Add header and footer content
Comment Actions
Change #1122672 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):
[design/codex@main] Popover: Update header styles per design feedback
Comment Actions
@DTorsani-WMF The latest patch on this task implements these bits of design feedback:
- There should be 8px of space between title and close button.
- The icon and close button should be aligned to top of "header", along with the title.
- The close button should use a smaller container so to not take up extra space that the quiet button normally does, like in Dialog.
I think we should discuss the notion of adding an icon and close button that are aligned with the body content rather than the header. To make things that flexible, we'll need to consider how it'll work inside the component, and (more importantly) how it'll work for Codex users. We need to make sure the component API is predictable and intuitive. I think it might make sense to enable the addition of a close button without adding the whole header, but I'm not so sure about icon. Consider this workflow: you have a Popover with an icon, title, and close button. You remove the title. How does the body now align with the icon? Is the entire body moved over to the right to account for the icon? Or is it just before the first line of body text, with the other lines wrapping underneath it?
In that case, it would probably make more sense for the user to just add the icon in the body so they can format it however they want.
We could just forego the icon and title props altogether. But it's quite tricky to get the icon and close button vertically centered with the first line of text. We should handle that inside Codex if it's a common use case.
This is another issue where the Popover is simply meant to be more flexible than the Dialog, so we may want the component API to be different. Let's discuss in Slack or on a call sometime? I don't think it needs to block this task, but it should block MVP since we want the component API to be stable for MVP.
Comment Actions
Thanks @AnneT. The updates look good. I agree with your sentiment around the icon with body, and that if an icon is needed it could be added to the body. Happy to discuss more if needed, but also agree this should not block this task.
Comment Actions
Change #1122672 merged by jenkins-bot:
[design/codex@main] Popover: Update header styles per design feedback
Comment Actions
Change #1124526 had a related patch set uploaded (by LWatson; author: LWatson):
[mediawiki/core@master] Update Codex from v1.20.2 to v1.20.3
Comment Actions
Test wiki created on Patch demo by LWatson-WMF using patch(es) linked to this task:
http://patchdemo.wmcloud.org/wikis/1f500859d0/wiki/Main Page
Comment Actions
Change #1124526 merged by jenkins-bot:
[mediawiki/core@master] Update Codex from v1.20.2 to v1.20.3
Comment Actions
Test wiki on Patch demo by LWatson-WMF using patch(es) linked to this task was deleted: