bensonarafat / super_tooltip

SuperTooltip It is super flexible and allows you to display ToolTips in the overlay of the screen.
https://pub.dev/packages/super_tooltip
MIT License
141 stars 95 forks source link

Small improvements #24

Closed websters-dog closed 3 years ago

websters-dog commented 3 years ago

I expierenced some troubles with nested navigators, so I added ability to pass overlay parameter. Also I required dismissing tooltip on scroll. For this purpose I added SuperTooltipDismissBehaviour with onPointerDown Listener with translucent hit test.

escamoteur commented 3 years ago

I'm sorry but you seem to have reformated the whole file so that I can't see where you made which changes

escamoteur commented 3 years ago

Please change the PR so hat I can see the changes.

websters-dog commented 3 years ago

Sorry, I missed formatting setup in my IDE. I'll fix it

websters-dog commented 3 years ago

Done!

escamoteur commented 3 years ago

why would you handle tap differently from pointer down?

websters-dog commented 3 years ago

It's useful in situation, when tooltip with transparent backgournd placed in scroll. Tooltip can be silently hidden on pointer down event and scroll widget gets event and works as it's required.

escamoteur commented 3 years ago

but does pointer down also get called if you tap on a touchscreen? you mean you problem is you don't want to swallow the drag events?

websters-dog commented 3 years ago

Yes, I don't want to block other events on the screen, just silently hide tooltip. For this purpose I set HitTestBehavior.translucent for Listener. Probably names for SuperTooltipDismissBehaviour enum items are not clear and I should rename it.

escamoteur commented 3 years ago

but why would we then keep the onTap at all?

websters-dog commented 3 years ago

In some situations it can be useful. For example when background has color. I guess current users also can rely on such behaviour.

escamoteur commented 3 years ago

ah, right the onTap respects the backgound color and also the cutout of the background. I guess that's not possible with the listeners. isn't it possible to make a GestureDetector not swallow the event?

websters-dog commented 3 years ago

Yes, it's possible for GestureDetector not to swallow events. Do you mean that it's better to left only one event onPointerDown or onTap and add bool parameter for pointer events blocking?

escamoteur commented 3 years ago

I wonder if it makes sense ever to block the other events

websters-dog commented 3 years ago

In my case it doesn't, but in general I don't know, so I left both ways not to break previous behaviour

escamoteur commented 3 years ago

yeah then lets go that way. adding a bool parameter that blocks as default all then it's not a breaking change at all what do you think?

websters-dog commented 3 years ago

I think it's ok. I'll use Listener with onPointerDown to hide tooltip before any scroll can happen for any blocking parameter value. Or will it be better to use on GestureDetector + onTap when value is true and Listener + onPointerDown on false?

escamoteur commented 3 years ago

how does a listener behave if you have a cut-out in the background? when you tap this area we should not react

websters-dog commented 3 years ago

Sorry, I missed the cut-out, need to check it

websters-dog commented 3 years ago

I reworked PR to bool property blockOutsidePointerEvents. It's true by default. If blockOutsidePointerEvents set to false events go through background and if dismissOnTapOutside is also true tooltip disapprears onPointerDown, but when you tap on cut-out tooltip stays on screen.

escamoteur commented 3 years ago

looks got to me. I'm in the process of handing over maintenance of this package to some friends, so I want them also to have a look at this. I you are interested to also become a maintainer let me know. I will move the package soon over to the Flutter Community organisation.

escamoteur commented 3 years ago

ok, I have merged it now but this changes have to be reviews and ported to the V2 branch too