jasonkuhrt / react-popover

A smart popover component for React
600 stars 253 forks source link

Expose popover state.standing as a CSS class on Popover element #102

Closed matmalkowski closed 6 years ago

matmalkowski commented 7 years ago

Hey, I'm currently writing my own component on top of this one, to apply some custom styles to the popover. Only issue I'm facing is the tip/arrow thing. its easy to style with one color tooltips, but my case has a popover with a border. To workaround this, I'm creating a CSS triangle on top of the SVG Tip thats moved by 1 pixel to direction of where popover is standing (it creates 1px wide border-like look). It works great, when I specify explicitly the place where the tooltip must be rendered (as then my component can append correct class to the element) but it's impossible to work with proffered place - I cannot determine where popover gonna get rendered, so cannot append correct class name to it.

Could I implement optional prop (bool) that if passed, would append the state.standing as a css class to the Popover master element? It would help to solve my issue and wouldn't conflict with other usages by default

raulmatei commented 7 years ago

Hi,

I think you can do that without the CSS triangle. Add a border to the Popover-body and a stroke to Popover-tipShape:

.Popover-body {
  border: 1px solid red;
}

.Popover-tip {
  margin-top: -1px;
}

.Popover-tipShape {
  stroke: red;
  stroke-dasharray: 0, 0, 20;
}

image

matmalkowski commented 7 years ago

@raulmatei Oh, didn't realize that, but still there is one problem with that solution - we are moving the tip with -1 to overlap the body boarder - your example works only for 'above', and there is no way from css to know where the popover got rendered, so I still need exposed state to apply correct margin value to the tip

raulmatei commented 7 years ago

Yep, that's an issue, indicating current position through a CSS class will work better. Still you could "hack" it with attributes selectors, but it gets very complex and ugly. It can be a temporary solution if you are forced to do it:

/* left position */
.Popover-body[style*="order: -1"] + .Popover-tip[width="7"] {
    margin-left: -1px;
}

/* right position */
.Popover-body[style*="order: 1"] + .Popover-tip[width="7"] {
    margin-right: -1px;
}

/* below position */
.Popover-body[style*="order: 1"] + .Popover-tip[width="14"] {
    margin-bottom: -1px;
}

/* above position */
.Popover-body[style*="order: -1"] + .Popover-tip[width="14"] {
    margin-top: -1px;
}
matmalkowski commented 7 years ago

@raulmatei thanks for the workaround hack is CSS. Still, if that would be ok, I think I could easly add the few lines of code that append position class to the main element based on some prop like 'appendPopoverPositionClass'

jasonkuhrt commented 6 years ago

This feature is now available in https://github.com/littlebits/react-popover/releases/tag/0.4.18! Thanks @raulmatei for the support!