feat(popover): add guidelines and code for popover#245
Conversation
✅ Deploy Preview for industrial-experience ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds documentation for the new Popover component, including usage guidelines, code examples, and sidebar navigation. The review feedback highlights several style guide violations in the newly added markdown files. Specifically, the feedback advises on avoiding Oxford commas, using contractions like 'don’t', framing instructions as suggestions rather than commands, using 'e.g.' instead of 'for example', and adhering to standard section headings and formatting for options, behaviors, and Dos and Don’ts.
alexkaduk
changed the title
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the documentation for the new Popover component, including its overview, usage guide, code examples, and API references, and registers it in the Docusaurus sidebar. The review feedback highlights several style guide violations in the newly added documentation files, such as incorrect heading hierarchies in the code tab, the use of passive voice instead of active voice, and incorrect pluralization. Additionally, the feedback suggests reordering the sidebar entry alphabetically to maintain consistency with the overview page.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/components/popover/index.mdx`:
- Line 5: The frontmatter in the Popover component documentation contains an
empty `deprecated:` field that creates inconsistency with the rest of the
documentation, which presents the component as production-ready and fully
supported. Since there are no deprecation notices, migration guidance, or
warnings elsewhere in the documentation, remove the empty `deprecated:` field
from the frontmatter entirely to align the metadata with the actual status of
the component. If the component is actually deprecated, provide a clear value
and add appropriate deprecation guidance to the documentation body instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19819574-9d77-4365-84ca-c02cdeb44c29
📒 Files selected for processing (6)
.env.pullrequestdocs/components/overview.mddocs/components/popover/code.mdxdocs/components/popover/guide.mddocs/components/popover/index.mdxsidebars.ts
kathrinschalber
left a comment
kathrinschalber
left a comment
There was a problem hiding this comment.
Looks a lot worse than it is 😉 like it overall!
danielleroux
changed the title
| Options for `ix-popover-header`, `ix-popover-image`, `ix-popover-content` and `ix-popover-footer` are documented in the [Usage guide](./guide). | ||
| <PropsApi /> |
There was a problem hiding this comment.
API for other popover components missing header,content and footer
| ```html | ||
| <ix-button id="triggerId">Open</ix-button> | ||
| <ix-popover trigger="triggerId" placement="bottom" has-spike close-on-click-outside> | ||
| <ix-popover-header icon="info">Popover title</ix-popover-header> | ||
| <ix-popover-image src="…" alt="…"></ix-popover-image> | ||
| <ix-popover-content>Body text</ix-popover-content> | ||
| <ix-popover-footer alignment="vertical"> | ||
| <span slot="start">v1.0.0</span> | ||
| <ix-button variant="secondary">Cancel</ix-button> | ||
| <ix-button>Save</ix-button> | ||
| </ix-popover-footer> | ||
| </ix-popover> | ||
| ``` | ||
|
|
||
| - **Popover title** — default slot text of `ix-popover-header` (see example above). | ||
| - **`additional-items` slot** — optional content beside the title, e.g. `ix-pill`. | ||
| - **`start` slot** on `ix-popover-footer` — optional leading footer content. | ||
|
|
||
| Call `hidePopover()` on the parent `ix-popover` to dismiss the popover from custom footer actions: | ||
|
|
||
| ```js | ||
| document.getElementById('popover')?.hidePopover(); | ||
| ``` | ||
| Use `showPopover()` and `hidePopover()` on the `ix-popover` element to open or close it programmatically. | ||
| Options for `ix-popover-header`, `ix-popover-image`, `ix-popover-content` and `ix-popover-footer` are documented in the [Usage guide](./guide). | ||
There was a problem hiding this comment.
Targets mainly native DOM, which is usually not our main audience, also its showcase what already shown in the example above.
| ## Basic | ||
|
|
||
| <PopoverPlayground height="32rem" /> |
There was a problem hiding this comment.
The combination of header, content and footer, is no "basic" usage of the popover. I would suggest add example usage of popover as a "popover" with custom content and not just a example with the predefined components
| - Do use a short title and concise content so users can scan the message quickly | ||
| - Don’t use popovers for essential information that must be read before proceeding, use [modals](../modal) instead | ||
| - Don’t use popovers for selecting from a list of actions, use [dropdowns](../dropdown) instead | ||
| - Don’t allow multiple popovers to be open at the same time, close the current one before opening another |
There was a problem hiding this comment.
Are nesting popovers also affected by this discussion?
There was a problem hiding this comment.
This don't is a separate topic; we had an actual don't for nesting popovers but removed it to keep the list neat and tidy.
5 participants
💡 What is the current behavior?
The
ix-popovercomponent and its sub-components (ix-popover-header,ix-popover-image,ix-popover-content,ix-popover-footer) are available in iX, but the documentation site has no dedicated popover page. Users cannot find usage guidance, code examples, or API reference for the new component in the components section.🆕 What is the new behavior?
ix-popoverand its sub-componentsFiles added
docs/components/popover/index.mdxdocs/components/popover/guide.mddocs/components/popover/code.mdxFiles updated
sidebars.tsdocs/components/overview.md.env.pullrequest👨💻 Help & support
ix-popover-imageand updated preview examples with icon registration) is merged or rebuilt before merging this PR.Summary by CodeRabbit
Release Notes