Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[✨] Make Popover work without using a div #961

Open
imMadsen opened this issue Sep 13, 2024 · 1 comment
Open

[✨] Make Popover work without using a div #961

imMadsen opened this issue Sep 13, 2024 · 1 comment
Labels
COMMUNITY: PR is welcomed We think it's a good feature to have but would love for the community to help with the PR for it TYPE: enhancement New feature or request

Comments

@imMadsen
Copy link

Is your feature request related to a problem?

The issue is when using a Popover for a Context Menu in a table, the Popover.Root element wraps the contents in a div. Essentially making it unusable for tables. Maybe my examples below can help clarify the issue

Example

<table>
    <Popover.Root>
        <tr>
            <td>John Doe</td>
        </tr>
        <Popover.Panel id="my cool menu">
        </Popover.Panel>    
    </Popover.Root>
</table> 

Example (Rendered)

<table>
    <div>
        <tr>
            <td>John Doe</td>
        </tr>
        <div id="my-cool-ctx-menu">
        </div>
    </div>
</table> 

Describe the solution you'd like

Dream Scenario (Rendered)

<table>
    <tr>
       <td>John Doe</td>
       <div id="my-cool-ctx-menu">
       <div>    
    </tr>
</table> 

Describe alternatives you've considered

Maybe use a Polymorphic components like described in the Qwik Documentation here, or maybe one could use a fragment (I assume this is not done cause of some dom requirements stuff)

Additional context

No response

@imMadsen imMadsen added STATUS-1: needs triage This doesn't seem right TYPE: enhancement New feature or request labels Sep 13, 2024
@thejackshelton
Copy link
Collaborator

thejackshelton commented Sep 16, 2024

The popover does need to be on a dom element.

The problem with polymorphism is the TypeScript server gets way too slow once you get past four possible elements, that is if you want it to be typed to the table props immediately after doing as="div".

At least with what I've tried. If you'd like to take a stab at it happy to help get you started with the research I've done so far.

Also a question, what use case would you have for a table row that pops over a table element? Why not have the table element inside the popover? Why is the popup needed? Maybe a modal would be better suited for this?

@thejackshelton thejackshelton added COMMUNITY: PR is welcomed We think it's a good feature to have but would love for the community to help with the PR for it and removed STATUS-1: needs triage This doesn't seem right labels Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMMUNITY: PR is welcomed We think it's a good feature to have but would love for the community to help with the PR for it TYPE: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants