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

issue 12: fixed tooltip flicker; prevent tooltip run off screen #13

Merged
merged 2 commits into from
Dec 1, 2023
Merged

issue 12: fixed tooltip flicker; prevent tooltip run off screen #13

merged 2 commits into from
Dec 1, 2023

Conversation

glasheen99
Copy link

The heart of this change was implementing custom OverlayLayout to size and position tooltips without waiting an extra frame (if I understood the root cause correctly). I haven't witnessed any flicker in the test app, nor in my app, since this change.

I added more tooltips to the demo app. I can remove them if needed.

OverlayLayout: I added a parameter gapTooltipScreen to control how close tooltips can approach left and right side of screen, but I didn't expose this in calling composables. I can do so if you want.

It's still possible to specify tooltips so that they run off the top or bottom of the screen. This is a harder than preventing left and right "overrun"; I mentioned it briefly in the layout code.

… from running off the screen, in some cases
@glasheen99 glasheen99 mentioned this pull request Nov 27, 2023
@pseudoankit
Copy link
Owner

pseudoankit commented Nov 28, 2023

Hey thanks for raising the pr, will check this

import com.pseudoankit.coachmark.model.TooltipConfig

/** Containing composable must use these values for layoutId on current and previous tooltip. */
public enum class OverlayChildLayoutId { CURRENT, PREVIOUS }
Copy link
Owner

Choose a reason for hiding this comment

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

BTW isn't this layoutId for Tooltip, can we rename TooltipId
also do we need this as enum looks this can be a object and values as const
don't see any usage of having enums here

@pseudoankit
Copy link
Owner

I added a parameter gapTooltipScreen to control how close tooltips can approach left and right side of screen, but I didn't expose this in calling composables

please expose it let's give client flexibility

@glasheen99
Copy link
Author

Somehow I can't reply here on the comment you made re: the enum. I replied on it in the code listing.

@pseudoankit pseudoankit merged commit 58e0f29 into pseudoankit:master Dec 1, 2023
1 check passed
@pseudoankit
Copy link
Owner

Thanks for the pr 🙏 , merged

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.

2 participants