-
-
Notifications
You must be signed in to change notification settings - Fork 229
feat: usePopper Hook #343
feat: usePopper Hook #343
Conversation
d970fe9 to
8fc9186
Compare
This comment has been minimized.
This comment has been minimized.
8fc9186 to
023fa3f
Compare
This comment has been minimized.
This comment has been minimized.
719e5f4 to
fcd1259
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Looks great imo |
fcd1259 to
5b8e7e6
Compare
5b8e7e6 to
5ff59a9
Compare
5ff59a9 to
2cbede8
Compare
This comment has been minimized.
This comment has been minimized.
ec45cf1 to
21dc44b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
So, I suppose the PR is ready for a final review. (I need to fix the tests though) |
|
Tests fixed, I replaced Enzyme with react-testing-library since it seems to work better with modern React versions. I also added tests for the new hook. |
|
hi @FezVrasta, thank you for this amazing work! Question: why does in the CodeSandbox demo, the I also face the same thing in my local, but I don't know whether it's because of this lib, upstream repository, or I'm configuring something wrong, though I copy-pasted exactly the same code like in the documentation for |
|
@imballinst Popper only positions the arrow along a single axis in order to center it to the reference. For vertical placements it's the |
|
I’m sorry I’m from mobile so I can’t look into it, but make sure you are applying the correct CSS to position the arrow. Popper provides only the offset to center it. You can look at the popper.js.org tutorial |
|
@atomiks @FezVrasta Ah, okay! Got it. That should be enough as a clue for me. Thanks and sorry for the noise. Have a nice weekend! |
|
shouldn't |
|
|
|
Is the demo's
still applicable? I'm having some issues setting |
It should work with null elements |
|
It's working, it's just a silly example where the popper sits below the button 😅 |

CodeSandbox demo:
https://codesandbox.io/s/cold-night-u1oji