-
Notifications
You must be signed in to change notification settings - Fork 955
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
rx.tooltip doesn't work with rx.button when that rx.button has additional properties #3278
Comments
Thanks for the report. Seems to be caused by the level of isolation we add when components need to use State related stuff. rx.tooltip(
rx.icon_button(
"home",
disabled=True
),
content="Go to homepage"
) the tooltip will show up properly. Need to investigate on Radix side, it might be a bug from them. |
Trimmed down code that reproduce the issue : /** @jsxImportSource @emotion/react */
import { Fragment } from "react";
import {
Box as RadixThemesBox,
Tooltip as RadixThemesTooltip,
} from "@radix-ui/themes";
export function Box_f821d43fc5d8628e6aa9573e5ba02f7f() {
return <RadixThemesBox>{`home`}</RadixThemesBox>;
}
export default function Component() {
return (
<Fragment>
<RadixThemesTooltip content={`Go to homepage`}>
<Box_f821d43fc5d8628e6aa9573e5ba02f7f />
</RadixThemesTooltip>
</Fragment>
);
} Moving component directly in the same function where Tooltip is used make the code work: /** @jsxImportSource @emotion/react */
import { Fragment } from "react";
import {
Box as RadixThemesBox,
Tooltip as RadixThemesTooltip,
} from "@radix-ui/themes";
export default function Component() {
return (
<Fragment>
<RadixThemesTooltip content={`Go to homepage`}>
<RadixThemesBox>{`home`}</RadixThemesBox>
</RadixThemesTooltip>
</Fragment>
);
} |
After posting I went looking for a workaround with different components, but it seems to impact anything that would appear on hover, not just rx.tooltip. |
If there's any workaround for this, please let me know, even if it's hacky. My app is just a mockup, so I'm not worried about it being done "right". Even if there's some way to set the alt property and use default browser tooltips, that'd be good. |
from reflex.components.component import MemoizationLeaf
from reflex.components.base.fragment import Fragment
class IntegralFragment(Fragment, MemoizationLeaf):
pass
...
IntegralFragment.create(
rx.tooltip(
rx.icon_button(
"home",
on_click=rx.redirect("/"),
disabled=(State.router.page.path=="/")
),
content="Go to homepage"
),
), Don't ask me why this works, needs further investigation. But it does appear to workaround the problem. |
Okay this actually helped a lot radix-ui/primitives#1166 Turns out that Reflex's StatefulComponent memoization is not handling ref and prop forwarding, which Radix is depending on. If i change the code to be like this export const Button_4f9c226eb6db5a68f02ace30e6d02ba7 = forwardRef((props, forwardedRef) => {
const state = useContext(StateContexts.state)
return (
<button ref={forwardedRef} {...props}>
{`fooey ${state.router.session.client_token}`}
</button>
)
}) Then I wonder if it makes sense to eventually structure all of the memoized components in this way to maximize compatibility. |
@masenf what's that workaround doing? |
The workaround is making it so that child of the tooltip does not get memoized separately. The |
Describe the bug
Tooltips are not displayed for (icon_)buttons once that button has addition properties specified.
To Reproduce
Use the following code in a page. Hovering over the icon_button will not display the tooltip.
If you comment out the
on_click
anddisabled
lines, then the tooltip works.Expected behavior
The tooltip should still be displayed.
Specifics (please complete the following information):
The text was updated successfully, but these errors were encountered: