Skip to content

[Toolkit][Shadcn] Add popover recipe#3487

Open
Amoifr wants to merge 3 commits intosymfony:3.xfrom
Amoifr:feat/toolkit-shadcn-popover
Open

[Toolkit][Shadcn] Add popover recipe#3487
Amoifr wants to merge 3 commits intosymfony:3.xfrom
Amoifr:feat/toolkit-shadcn-popover

Conversation

@Amoifr
Copy link
Copy Markdown
Contributor

@Amoifr Amoifr commented Apr 18, 2026

Q A
Bug fix? no
New feature? yes
Deprecations? no
Documentation? no
Issues Part of #3233
License MIT

Adds the popover recipe to the Shadcn kit.

Part of the split of #3468 into one PR per component, tracking #3233.

Snapshots will be regenerated after rework.

@Amoifr
Copy link
Copy Markdown
Contributor Author

Amoifr commented Apr 20, 2026

Main example

Capture.video.du.2026-04-20.08-41-21.mp4

@Amoifr
Copy link
Copy Markdown
Contributor Author

Amoifr commented Apr 20, 2026

Basic example

Capture.video.du.2026-04-20.08-42-25.mp4

@Amoifr
Copy link
Copy Markdown
Contributor Author

Amoifr commented Apr 20, 2026

Align example

Capture.video.du.2026-04-20.08-47-25.mp4

@Amoifr
Copy link
Copy Markdown
Contributor Author

Amoifr commented Apr 20, 2026

With form example

Capture.video.du.2026-04-20.08-53-15.mp4

@Amoifr
Copy link
Copy Markdown
Contributor Author

Amoifr commented Apr 20, 2026

RTL example

Capture.video.du.2026-04-20.09-01-46.mp4

@Amoifr Amoifr force-pushed the feat/toolkit-shadcn-popover branch from 7129082 to d55e111 Compare April 20, 2026 07:04
@Amoifr Amoifr marked this pull request as ready for review April 20, 2026 07:05
@Amoifr Amoifr requested a review from Kocal as a code owner April 20, 2026 07:05
@carsonbot carsonbot added Feature New Feature Toolkit Status: Needs Review Needs to be reviewed labels Apr 20, 2026
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By doing this, you are forcing the user to use a button like as a trigger, this is not our philosophy.

Please check the existing *Trigger.html.twig, they are nearly-empty templates that expose a Twig variable ..._trigger_attrs, e.g. with AlertDialog/Trigger.html.twig:

{# @block content The trigger element (e.g., a `Button`) that opens the dialog when clicked #}
{%- set alert_dialog_trigger_attrs = {
    'data-action': 'click->alert-dialog#open'|html_attr_type('sst'),
    'data-alert-dialog-target': 'trigger',
    'aria-haspopup': 'dialog',
    'aria-expanded': 'false',
} -%}
{%- block content %}{% endblock -%}

Usage:

<twig:AlertDialog id="delete_account">
    <twig:AlertDialog:Trigger>
        <twig:Button variant="outline" {{ ...alert_dialog_trigger_attrs }}>
            Show Dialog
        </twig:Button>
    </twig:AlertDialog:Trigger>
    <twig:AlertDialog:Content>...</twig:AlertDialog:Content>
</twig:AlertDialog>

It also means that you must not use details and summary HTML elements.

Thanks!

@carsonbot carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Apr 23, 2026
Amoifr added a commit to Amoifr/ux that referenced this pull request Apr 23, 2026
Follow up on the review feedback on symfony#3487: the Trigger template
should not enforce a specific HTML element but expose a Twig
variable with the needed attributes so the consumer can pick
their own element (button, link, span...).
Amoifr added a commit to Amoifr/ux that referenced this pull request Apr 23, 2026
Follow up on the review feedback on symfony#3487: the Trigger and SubTrigger
templates should not enforce a specific HTML element but expose a Twig
variable with the needed attributes so the consumer can pick their own
element.
Amoifr added a commit to Amoifr/ux that referenced this pull request Apr 23, 2026
Follow up on the review feedback on symfony#3487: the Trigger template
should not enforce a specific HTML element but expose a Twig variable
with the needed attributes so the consumer can pick their own element.
Follow up on the review feedback on symfony#3487: the Trigger template
should not enforce a specific HTML element but expose a Twig
variable with the needed attributes so the consumer can pick
their own element (button, link, span...).

Since the previous implementation relied on native
`<details>`/`<summary>` to handle open/close, replace it with a
`<div>` backed by a new `popover_controller.js` Stimulus
controller that handles toggle, outside click, escape and
mutual exclusion by `name`.

Also drop two orphan snapshots (`Demo.html__1.html`,
`Usage.html__1.html`) shipped by mistake and no longer
generated by the test suite.
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Apr 23, 2026
@Amoifr Amoifr marked this pull request as draft April 23, 2026 11:10
@Amoifr
Copy link
Copy Markdown
Contributor Author

Amoifr commented Apr 23, 2026

Reworked to apply the _trigger_attrs convention (from your review on this very PR): swapped native <details>/<summary> for a <div> + a small Stimulus controller so the Trigger stays element-agnostic. Moving back to draft so you can re-test at your convenience. Thanks for the patience! 🙏

Amoifr added a commit to Amoifr/ux that referenced this pull request Apr 23, 2026
Follow up on the review feedback on symfony#3487: the Trigger template
should not enforce a specific HTML element but expose a Twig
variable with the needed attributes so the consumer can pick
their own element (button, link, span...).

Since both `Drawer` and `Sheet` relied on native
`<details>`/`<summary>` to handle open/close, replace them
with `<div>`s backed by small `drawer_controller.js` and
`sheet_controller.js` Stimulus controllers handling toggle,
outside click and escape.

Also drop four orphan snapshots (`Demo.html__1.html`,
`Usage.html__1.html` for both kits) shipped by mistake and no
longer generated by the test suite.
Amoifr added a commit to Amoifr/ux that referenced this pull request Apr 23, 2026
Follow up on the review feedback on symfony#3487: the Trigger template
should not enforce a specific HTML element but expose a Twig
variable with the needed attributes so the consumer can pick
their own element (button, link, span...).

Since `DropdownMenu` relied on native `<details>`/`<summary>`
to handle open/close, replace it with a `<div>` backed by a
new `dropdown_menu_controller.js` Stimulus controller handling
toggle, outside click and escape.

Convert `SubTrigger` to the same convention (drop the hardcoded
`<button>`/chevron; consumer now picks the wrapping element
and adds the chevron). Migrate `Sub`/`SubContent` from the
`peer/peer-hover` pattern to `group/group-hover` so the sub-menu
still opens on hover once the peer anchor moves out of the
template.

Also drop two orphan snapshots (`Demo.html__1.html`,
`Usage.html__1.html`) shipped by mistake and no longer
generated by the test suite.
Amoifr added a commit to Amoifr/ux.symfony.com that referenced this pull request Apr 23, 2026
Follow-up of symfony/ux#3487: the Popover recipe now ships a
Stimulus controller, register it in the toolkit-shadcn entrypoint
and add the corresponding importmap entry so examples on this
docs page work interactively.
@Amoifr Amoifr marked this pull request as ready for review April 23, 2026 14:41
@Amoifr
Copy link
Copy Markdown
Contributor Author

Amoifr commented Apr 23, 2026

Re-tested locally on ux.symfony.com (with the matching controller registration on symfony/ux.symfony.com#70) — open/close, outside click and escape all behave as expected on every example (Demo / Usage / Basic / Alignments / Form / RTL). Back to ready for review. 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature New Feature Status: Needs Review Needs to be reviewed Toolkit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants