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

Fix Tooltip inside Scrollable #1405

Merged
merged 2 commits into from
Aug 17, 2022

Conversation

tarkah
Copy link
Member

@tarkah tarkah commented Aug 9, 2022

Tooltip inside Scrollable is currently broken since the viewport passed to the Tooltip is modified by Scrollable to be it's visible bounds instead of the application viewport bounds. This causes the Tooltip to position itself within these bounds.

I've added a builder to Tooltip to disable the positioning adjustment that ensures its always inside the viewport with snap_within_viewport(bool). I've then also removed the layer clip when drawing the tooltip so it's not clipped by the scrollable bounds (tooltip is effectively an overlay and should never be clipped).

Commit b3f5f74 shows the bug and 046e7e0 shows it being fixed. These commits can get dropped.

I realize this is a hacky solution, since snap_within_viewport shouldn't be something that's exposed or even considered and isn't really honest about why it's there. However, since we don't get the actual window viewport in the widget, we have no way of know when or when not to adjust to the viewport.

Should we do something like the following, so there is a window bounds that can be used that we know is unchanged during draw. Tooltip can then use this for it's positioning adjustment to the window bounds.

fn draw<'a>(
    &'a self,
    ...,
    viewport: &Rectangle,
    window: &'a Rectangle,
)

@hecrj hecrj added feature New feature or request widget bug Something isn't working labels Aug 17, 2022
@hecrj hecrj added this to the 0.5.0 milestone Aug 17, 2022
@hecrj hecrj force-pushed the fix/tooltip-inside-scrollable branch from 046e7e0 to 1ae3a94 Compare August 17, 2022 14:16
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Thanks! This should be good enough until we generalize our overlay support.

@hecrj hecrj merged commit bf7ea81 into iced-rs:master Aug 17, 2022
tarkah pushed a commit to tarkah/iced that referenced this pull request Aug 17, 2022
tarkah pushed a commit to tarkah/iced that referenced this pull request Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature New feature or request widget
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants