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

Default styling is very broken #69

Open
LizAinslie opened this issue Jun 30, 2022 · 8 comments
Open

Default styling is very broken #69

LizAinslie opened this issue Jun 30, 2022 · 8 comments

Comments

@LizAinslie
Copy link

LizAinslie commented Jun 30, 2022

Description of the bug
I'm using this component as a child of a flex container, and I found it's broken all of my navbar styles as well as causes horizontal overflow on the body, which is not acceptable. Screenshot:
https://i-work-at-the.cocaine.institute/Lizzy62bd8388RPcYquNqXKzV.png

For some reason it wraps the trigger in a component that gets 5px added to its height, relevant screenshot:
https://i-work-at-the.cocaine.institute/Lizzy62bd84147poIQcPCnp9s.png

And applies an awful border to handle spacing (wtf?)
https://i-work-at-the.cocaine.institute/Lizzy62bd84a0vf0mW0ErlITY.png

All of this is clearly hacky and bad, and this library would be far more useful to me (and many other users, as apparently there are two separate issues open regarding the removal of wrapper divs) if it didn't attempt to provide such default styles and instead wrapped popper.js sanely for vue 3, which for some reason is not being done by any library I've found. This is my closest bet.

I managed to halfway fix it by adding the following to my stylesheet:

.inline-block {
  margin: 0 !important;
  border: none !important;
}

and adding a -5px margin to the trigger child.

However, in doing so it seems to have completely broken the offset-skid and the popover no longer respects that option.

Reproduction link
If you can, create a reproduction on CodeSandbox

To Reproduce

This is the code i'm using.

<Popper
  offset-distance="16"
  offset-skid="12"
>
  <img
    class="avatar"
    src="https://cdn.discordapp.com/avatars/543542278967394322/99f5040863c94823e743134348722b1c.png"
    alt="My Profile Picture"
  >

  <template #content>
    <div><!-- popup content --></div>
  </template>
</Popper>

The parent has the following styles applied:

display: flex;
align-items: center;
justify-content: end;
padding: 8px 12px;

Expected behavior
I expect this library to properly handle spacing and not break my layout. I also expect it to not use hacky solutions like a 16px transparent border with a -16px margin for god knows what reason.

Screenshots
Added above.

Additional context
Add any other context about the problem here.

@SteiniDJ
Copy link

What's up with the tone here? It probably does very little to encourage the maintainers. Here's a reminder: You're not being forced to use the package and are free to fork it and change it as you see fit.

@LizAinslie
Copy link
Author

No negative tone was intended, sorry. I was simply taken aback by a few of the weird hacks I saw and that may have shown in my writing. It's difficult to convey tone over a medium that doesn't have inflection, such as text. That's why tone indicators exist, for people such as myself who already have a hard time reading tone in day-to-day conversation and for which the internet would be far more difficult.

Anyway, I've resorted back to a non-popper solution as I can't seem to find a good, working Vue 3 implementation that doesn't mess with styles weirdly. I really want to use this lib as the structure makes sense but I can't with it in this state.

@prattcmp
Copy link

prattcmp commented Sep 2, 2022

+1

@Kurohyou
Copy link

Seconding the need to step away from the negative margins and borders. Base popper uses translate3d, which I had assumed was just part of the popper library. Perhaps relying on that for the offset distance and skid would be a better option?

@LizAinslie
Copy link
Author

LizAinslie commented Nov 19, 2022

This, this, this. If the library you're wrapping already provides a good solution why reinvent the wheel?

Anyway bump I guess. I think this lib is dead/unmaintained which is sad :/

@LizAinslie
Copy link
Author

Apparently according to #30 and the conversation involved the maintainer intends to keep this behavior unless it is causing problems, which it very clearly is. @valgeirb I'm pinging you to inform you that this is still an issue as stated in #30, #45, and #50 as well as here, and is still causing major problems for many people. Surely there is a better way to go about this than styling the attached element. My schedule is booked or I'd submit a PR resolving all these issues.

@Kurohyou
Copy link

Kurohyou commented Nov 19, 2022 via email

@LizAinslie
Copy link
Author

Now that I think about it with a bit more clarity I could have probably wrapped it myself and just dropped the flag (it's also been a couple of months since I've touched the project in question so take what I say with a grain of salt here) The problem is that I feel I shouldn't have to do that if I want proper interactivity without fucking my UI up.

Anyway I just hope our lovely maintainer sees this and cares enough to maybe offer a solution. A lot of Vue libraries are like this, opinionated and broken, usually in ways that make sense for one or two specific use cases but not as a general purpose solution. Then, when it's brought up as an issue it gets closed as intended behavior. I guess I'm just too used to the react way of life xD

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

No branches or pull requests

4 participants