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

support top & left arrow offsets together #37

Merged
merged 1 commit into from Jul 19, 2017

Conversation

giladgray
Copy link
Contributor

fixes #34

} else {
return { left: +left }
}
return { top: +top, left: +left }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 @FezVrasta should I include bottom and right here too? feels like a yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nvm, not necessary

@FezVrasta
Copy link
Member

I'm not at pc but I thought the computeStyle modifier created an object with any needed style prop just like it happens for the popper element. This way you don't need to manually specify any

@giladgray
Copy link
Contributor Author

@FezVrasta computeStyle does not produce an object for arrow, only for popper
floating-ui/floating-ui#347

@FezVrasta
Copy link
Member

My lib needs some refactor/cleaup so bad.. 🙄

@giladgray
Copy link
Contributor Author

ok @souporserious i think this PR is sufficient for now.
the next iteration will wait for floating-ui/floating-ui#347.

@giladgray
Copy link
Contributor Author

@souporserious what's the path to merging this and #40? i'll need a release containing both of them to unblock some of my other feature work. happy to help however I can!

@souporserious
Copy link
Collaborator

Sorry been super busy this last week. Looking at everything now :) thanks for all of your work! I appreciate it.

@souporserious souporserious merged commit c934718 into floating-ui:master Jul 19, 2017
@giladgray giladgray deleted the arrow-offsets branch July 19, 2017 18:06
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

Successfully merging this pull request may close these issues.

allow Arrow to offset both left and top
3 participants