-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
anchorEl prop for popover ? #2866
Comments
@suhail-ansari-apconic the |
Popover needs something to position itself to, which is always a DOMElement, as it calls In my view, it's easier for popover to take the DomElement an position itself, versus being passed a position directly. Because the element is being passed in, it will reside on state external to the popover itself, so popover can't manage that memory itself. Perhaps popover could have an inline "auto anchor" element, e.g. an empty span, which renders wherever popover is declared in the markup. This might make the anchorEl redundant in some cases. Or alternatively, we could have anchorEl take a function a la react-bootstrap-overlays... |
The current approach, with the DOMElement being provided for the popover to compute the position, breaks when wanting to provide a popover for a grid cell (popover declared outside the grid, with only one cell active). Why? This is because the target element most probably needs to be saved in the state, which in turn triggers a redraw and the DOMElement no longer exists when given to the popover which will give a bounding rectangle with all properties set to 0. If, instead, ReactDOM.findDOMNode was called on the component (which was saved in the state on the click event on itself) in the render method, that would trigger a "findDOMNode was called on an unmounted component." error, because that component was re-rendered and the reference is old. In such cases, one would prefer giving the bounding rectangle to the popover instead of the target element (and storing that instead of the DOMElement), as providing an old DOMElement will give bounding rectangle with all properties set to 0. If you think of any workaround for the above situation, please let me know. |
We have been porting the component on the v1-beta branch. We reimplemented it from the ground-up. While we haven't tested it, I think that the issue is most likely fixed on that branch. Hence, I'm closing it. |
It looks like Popover was moved to the package "internal", so I don't know, if it should be used any longer. But it still has the same properties as on the master branch (including The new example for Simple Menus on https://material-ui-1dab0.firebaseapp.com/component-demos/menus also contains code with So I think that the original problem is still unsolved. |
@planetrenner-martin We plan on exposing the Popover component #6270. Noticed that it's rendering a backdrop on the page, for some situation you might want to use react-popper instead. |
Popover anchor to I know it works in v1 beta according to the website. However I'm still using material-ui v0.20.0. (seems like a lot of changes need to be made to migrate to v1-beta. for example, |
I'm having the same problem than you @bfang711, does anyone know a solution for that? |
Hey guys, I got the impression that the positioning of the element does not work since the anchor element is re-rendered and is not yet placed into the real dom. That is why the Rect data is 0, 0, 0 and 0. :) I'm gonna try to avoid the re-rendering of the anchor element to check if this is the root cause. Update 1: Update: 2: |
Yes, but the property anchorPosition is only available in the v1.0 (next) material-ui implementation. Some of us are still using the v0.20 (master) implementation, which makes it difficult to use the Popover when re-rendering of the target element occurs and the element is not in the dom, making the rectangle (0, 0, 0, 0). Maybe it would be nice to provide such a property, like anchorPosition, to manage the positioning rectangle from the outside of the component, to the master implementation as well? |
I'm not sure if my situation was exactly the same as above, but to resolve the
|
check this it will help you - https://codesandbox.io/s/use-ref-for-anchorel-forked-1pvh1 |
Passing it a ref to the component like Appbar as anchorEl does not work. Now i have not seen any documentation regarding what kind of value anchorEl needs. According to react documentation they discourage use of findDOMNode etc as it breaks encapsulation so refs should work.
The text was updated successfully, but these errors were encountered: