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

[EuiPopover] Are 2 wrapper components necessary? #2022

Closed
Tracked by #4054
cchaos opened this issue Jun 10, 2019 · 8 comments · Fixed by #7311
Closed
Tracked by #4054

[EuiPopover] Are 2 wrapper components necessary? #2022

cchaos opened this issue Jun 10, 2019 · 8 comments · Fixed by #7311
Assignees

Comments

@cchaos
Copy link
Contributor

cchaos commented Jun 10, 2019

<div class="euiPopover">
  <div class="euiPopover__anchor">
    <button />
  </div>
</div>

I remember we were originally positioning the popover with CSS making the anchor wrapper necessary, but is it still?

@snide
Copy link
Contributor

snide commented Jun 11, 2019

Given that the popover itself has inline block, my guess is the __anchor is no longer needed.

@thompsongl
Copy link
Contributor

It's not needed for layout or style as far as I can tell. However, we use it to host a ref and allow consumers to pass classes directly to it.
We'd need to do the legwork to verify that any classes currently being passed could stand to be passed instead to the button (or are needed at all).
We'd also need to update EuiButton to use forwardRef for the default case and indicate that any custom anchor passed to EuiPopover also needs to be able to accept a ref.

@cchaos
Copy link
Contributor Author

cchaos commented Jul 11, 2019

I just find myself needing a blocked display of the popover and having to give to classes with display: block in order to do so. Always feels so cumbersome. However, I already got so fed up that I just created a prop #2112

@cchaos
Copy link
Contributor Author

cchaos commented Sep 18, 2020

I'd like to make this a long-term goal. It's something that I continue to fight with constantly. This and the EuiTooltip.

@cchaos cchaos changed the title [Question] Are 2 wrapper components necessary with EuiPopover [EuiPopover] Are 2 wrapper components necessary? Sep 20, 2020
@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@github-actions
Copy link

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

@github-actions
Copy link

❌ Per our previous message, this issue is auto-closing after having been open and inactive for a year. If you strongly feel this is still a high-priority issue, or are interested in contributing, please leave a comment or open a new issue linking to this one for context.

@cee-chen
Copy link
Contributor

I'm going to re-open this as this annoys me personally 😅

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

Successfully merging a pull request may close this issue.

4 participants