feat(popover): add docs IX-3942#245
Conversation
✅ Deploy Preview for industrial-experience ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
|
/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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughAdds complete documentation for the ChangesPopover Component Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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
| doc-type: "tabs" | ||
| description: 'Popovers display contextual information in a floating panel anchored to a trigger element.' | ||
| title: 'Popover' | ||
| deprecated: |
There was a problem hiding this comment.
Remove or clarify the empty deprecated: field.
The frontmatter contains an empty deprecated: field (line 5) with no value. This is inconsistent with the component's documentation:
- The guide and overview both present Popover as a fully-supported, production-ready component.
- The "Dos and Don'ts" section treats it as a viable option for contextual information.
- There is no deprecation notice, migration path, or warning elsewhere in the documentation.
Either remove the deprecated: field if it is not deprecated, or provide a value and deprecation guidance if it is. As per coding guidelines, metadata should be clear and complete.
🤖 Prompt for 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.
In `@docs/components/popover/index.mdx` at 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.
Source: Coding guidelines
kathrinschalber
left a comment
There was a problem hiding this comment.
Looks a lot worse than it is 😉 like it overall!
| --- | ||
| # Popover - Usage | ||
|
|
||
| Use popovers when users need extra context without leaving the current task or losing sight of the trigger. We recommend popovers for short, contextual content that helps users decide, confirm or learn more in place. |
There was a problem hiding this comment.
"decide": You might reframe this a bit
- In the dos section you recommend only 1 action
- up till now, decisions are done in modal messages - do we now recommend both? (if so, I'd add a related-link to the message modal)
There was a problem hiding this comment.
I will change "decide" to "act" to keep the guidance for important decisions in the modal message. Focus here should be that popovers provide help to those actions.
| - **Footer actions:** Add footer actions only when users need a lightweight next step, e.g. dismissing a hint or opening more details | ||
| - **Alignment:** Use horizontal footer actions by default. Switch to vertical only when labels are long or space is constrained | ||
|
|
||
| ## Behavior in context |
There was a problem hiding this comment.
we typically use this section not for recommendations but how the component behaves in context of other elements (e.g. keep text concise should be placed on the title). As a result, what do you think about describing the focus-logic here?
- if there are interactive element inside: focus goes from trigger element to first interactive element, when closing back to trigger element
- if there are no interactive elements: focus stays on trigger
There was a problem hiding this comment.
removed or the recommendation style and added the focus logic
| - **Trigger:** Use a clear trigger that signals additional context, e.g. an info icon, status chip or secondary action | ||
| - **Placement:** We typically use the default bottom placement because it feels connected to the trigger and works well in dense layouts | ||
| - **Has spike:** Use the spike when users need a stronger visual connection between the popover and its trigger | ||
| - **Trigger mode:** Prefer click for interactive content. Use hover only for lightweight, non-essential information |
There was a problem hiding this comment.
The hover-definition basically describes tooltips, isnt it?
| - Do use popovers for contextual information, e.g. release highlights or guided hints | ||
| - Do use popovers when users need one lightweight action without leaving the current task | ||
| - Do use a short title and concise content so users can scan the message quickly | ||
| - Do use tooltips for brief, non-interactive hints on hover or focus |
There was a problem hiding this comment.
Would it make sense to add a writing tab, containing the infotip guidelines? https://ix.siemens.io/docs/guidelines/language/messaging/infotips
Might only need some introductory sentence about the "special" naming
| - **Close on click outside:** Keep this enabled for temporary, dismissible information so users can return to the page quickly | ||
| - **Title:** Add a short title when users need to understand the topic before scanning the content | ||
| - **Image:** Use images sparingly and only when they clarify the message faster than text alone | ||
| - **Footer actions:** Add footer actions only when users need a lightweight next step, e.g. dismissing a hint or opening more details |
There was a problem hiding this comment.
In other systems, stacked or numerous actions are usually treated as a warning sign, not just a spacing choice - consider defining how many actions are acceptable
|
|
||
| Popovers have two states: Closed and opened. When a popover is opened, it appears anchored to the trigger and is dismissible so users can return to the underlying context quickly. | ||
|
|
||
| ## Dos and Don’ts |
There was a problem hiding this comment.
How about multiple popovers at once? Or nesting dropdowns (e.g. a select or date input)?
| - **Title:** Add a short title when users need to understand the topic before scanning the content | ||
| - **Image:** Use images sparingly and only when they clarify the message faster than text alone |
There was a problem hiding this comment.
Consider be a bit more concrete, e.g.
titles: specific and scannable
images: never be decorative in a small transient overlay, no text - add alt-texts
| 1. Header with popover title (optional) | ||
| 2. Image (optional) | ||
| 3. Content | ||
| 4. Footer (optional) |
There was a problem hiding this comment.
I wasnt aware that we add the information about optional 🙊 we might need to add that to several components if needed
| 4. Footer (optional) | ||
| 5. Spike (optional) | ||
|
|
||
| ## Options |
There was a problem hiding this comment.
header icon property is missing; also consider additional items slot and start slot inside the footer
💡 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
ix-popover.