Add option to customize the skrim color

[caseycrogers]

It'd nice to be able to partially obscure the content behind the tooltip to communicate that tapping outside the tooltip will dismiss the tooltip.

Ideally, we'd also provide the option to not obscure the child widget with the colored skrim-but I honestly have no idea how to do that. We'd have to get the outline of the child widget and use it as a clip path on the skrim.

Leaving it to you to bump the version numbers and changelog should you accept this PR.

[stargazing-dino]

I like this. I would go one step further though and allow arbitrary content as the scrim. You can do this by defining a builder and letting the user pass through a function that builds their scrim widget. Maybe something like scrimBuilder? See here for another builder example I have.

It would then look like:

JustTheTooltip(
  scrimBuilder: (context) {
    return Container(color: Colors.blue.withAlpha(.5));
  }
)

What do you think of this?

Also,

Ideally, we'd also provide the option to not obscure the child widget with the colored skrim-but I honestly have no idea how to do that. We'd have to get the outline of the child widget and use it as a clip path on the skrim.

I'm not sure what you mean by this? Does this happen? My immediate thought is that the tooltip is above the scrim no? I'm not at simulator to test though

[caseycrogers]

Done! I made a couple value judgements, let me know if you think any of them should have been done differently:

1. Instead of replacing barrierColor, I added another argument for the builder. This way if a user just wants to change the color and that's it, they don't have to have deal with the added complexity of the builder.
2. I named the builder barrierBuilder instead of scrimBuilder to keep the external facing naming consistent with barrierColor and barrierDismissable.
3. Instead of putting the user-supplied barrier under the existing gesture detector for dismissing the barrier, I passed the _hideTooltip function into the barrierBuilder and left it to the developer to call the dismiss function as they see fit. This allows for arbitrarily customizable UX for dismissing the tooltip from the barrier.

[caseycrogers]

Hmm after doing a lot more work on #49, I'm realizing this PR may not add much value. Namely, the example file in #49 shows how to trivially build a customizable barrier. If it's so easy to build one outside of the package, then I don't think we need barrierBuilder in the package.

barrierColor still seems useful for people who want good out-of-the-box behavior and don't want to go through the trouble of building their own barrier just to customize the color.

My vote is rollback the barrierBuilder argument and keep barrierColor. Let me know what you think.

[stargazing-dino]

I like your example you have in #49 but I'm still a little unsure. I think I prefer having both barrierBuilder and barrierColor.

The example you have relies on aligning an external _animation with the internal one and using that animation inside a stack (which some users might not like being a part of the tree if not required to) to drive a transition.

Instead, I think I'd prefer the barrierBuilder to just be passed the internal animation that we already have for fading in and out the tooltip. I don't see a lot of use cases requiring a separate animation with a different duration but if that is the case, I think down the line we can create arguments like barrierShowDuration, etc.

(I also think it's possible to remap an animation to a different duration or curve? not sure)

The signature for barrierBuilder would then be:

typedef BarrerBuilderCallback = Widget Function(BuildContext context, Animation<double> animation, VoidCallback dismiss);

and the example would then be:

JustTheTooltip(
  barrierBuilder: (context, Animation<double> animation, onDismiss) {
    return LayoutBuilder(
      builder: (context, constraints) {
        // Use the pythagorean theorem to get the diameter of the circle
        // that circumscribes the constraints.
        final double diameter = math.sqrt(
            math.pow(constraints.maxHeight, 2) +
                math.pow(constraints.maxWidth, 2));

        return ScaleTransition(
          scale: animation,
          child: OverflowBox(
            maxWidth: diameter,
            maxHeight: diameter,
            child: Container(
              decoration: const BoxDecoration(
                color: Colors.white30,
                shape: BoxShape.circle,
              ),
            ),
          ),
        );
      },
    );
  },
)

I think the default barrierBuilder should just be null so we could use the barrierColor instead when building the scrim.

Widget _createSkrim() {
  if (widget.barrierBuilder != null) {
    return widget.barrierBuilder(context, animation, _hideTooltip);
  }
  
  return Container(
    color: barrierColor, // defaults to Colors.transparent
    child: GestureDetector(
      key: skrimKey,
      behavior: barrierDismissible
          ? HitTestBehavior.translucent
          : HitTestBehavior.deferToChild,
      onTap: _hideTooltip,
    ),
  );
}

I really appreciate the work you've put into this too !

[caseycrogers]

I like your example you have in #49 but I'm still a little unsure. I think I prefer having both barrierBuilder and barrierColor.

The example you have relies on aligning an external _animation with the internal one and using that animation inside a stack (which some users might not like being a part of the tree if not required to) to drive a transition.

Instead, I think I'd prefer the barrierBuilder to just be passed the internal animation that we already have for fading in and out the tooltip. I don't see a lot of use cases requiring a separate animation with a different duration but if that is the case, I think down the line we can create arguments like barrierShowDuration, etc.

(I also think it's possible to remap an animation to a different duration or curve? not sure)

The signature for barrierBuilder would then be:

typedef BarrerBuilderCallback = Widget Function(BuildContext context, Animation<double> animation, VoidCallback dismiss);

and the example would then be:

JustTheTooltip(
  barrierBuilder: (context, Animation<double> animation, onDismiss) {
    return LayoutBuilder(
      builder: (context, constraints) {
        // Use the pythagorean theorem to get the diameter of the circle
        // that circumscribes the constraints.
        final double diameter = math.sqrt(
            math.pow(constraints.maxHeight, 2) +
                math.pow(constraints.maxWidth, 2));

        return ScaleTransition(
          scale: animation,
          child: OverflowBox(
            maxWidth: diameter,
            maxHeight: diameter,
            child: Container(
              decoration: const BoxDecoration(
                color: Colors.white30,
                shape: BoxShape.circle,
              ),
            ),
          ),
        );
      },
    );
  },
)

I think the default barrierBuilder should just be null so we could use the barrierColor instead when building the scrim.

Widget _createSkrim() {
  if (widget.barrierBuilder != null) {
    return widget.barrierBuilder(context, animation, _hideTooltip);
  }
  
  return Container(
    color: barrierColor, // defaults to Colors.transparent
    child: GestureDetector(
      key: skrimKey,
      behavior: barrierDismissible
          ? HitTestBehavior.translucent
          : HitTestBehavior.deferToChild,
      onTap: _hideTooltip,
    ),
  );
}

I really appreciate the work you've put into this too !

All this sounds good, I'll make the changes now. And no problem!

[caseycrogers]

One thing: the example in #49 is not achievable with barrier builder. Specifically, the example draws a barrier behind the button that generated the tooltip. With barrier builder, the barrier is displayed in front of the button.

Personally I dramatically prefer the former, but it seems fine to provide both options?

[stargazing-dino]

Ah sorry, I understand what you mean now when you said the barrier will be drawn above the child button and we should look into a clipper or similar.

Looking for other examples on the web, it doesn't sound reasonable to have the barrier obscure the child which is what we do now. Up till now it hasn't been a problem since the barrier was always transparent.

Honestly, I still think the barrierBuilder is the way to go as the alternative seems like a lot of work for a user to implement themselves that should be taken care of at the library level.

I want to hear your opinion first but what do you think about moving forward with what we currently have (current issue and all) and then creating a seperate issue the deals with the barrier being above the child.

Namely, one idea I have is that we'd need to make the child an overlay too and then place the overlay at the end of the overlay stack:

[   content overlay   ]
[   barrier  overlay  ]
[   child overlay     ]

That or some thing else involving a simple clip space with the option to provide a custom clipper but this seems like a big enough thing to be its own issue. Lemme know

[caseycrogers]

I think proceeding as is and then trying to fix it after is a good approach-this isn't a world ending bug/UX issue.

Putting the child in an overlay doesn't really work because overlay operates in a separate context and developers using the API might have dependencies on context that are broken in overlay. I think the approach will have to be something painful like looking into shadermasks or something to see if we can clip the barrier to the inverse of the shape of the child.

[stargazing-dino]

Yah, it probably won't be fun but I'm sure someone must've tread this water before us. I know coachmaker does something similar to what we want so maybe they have an answer. Anyways, yah, we can move further discussion of this to #50 and refocus on getting this pr sent thru. Thanks for your patience with me too

[PiotrMitkowski]

Hey! I see that you both agreed on using implementation from this PR for now and work on not displaying the skrim over the child. A separate issue for the latter part has also been created. I'll merge this PR then.