search

goetzrobin / spartan

Cutting-edge tools powering Angular full-stack development.
https://spartan.ng
MIT License
1.1k stars 121 forks source link

Easier usage of tooltip #222

Closed alexciesielski closed 3 months ago

alexciesielski commented 3 months ago

Which scope/s are relevant/related to the feature request?

tooltip

Information

Currently to show a tooltip on a button I:

  1. need to wrap it with a hlm-tooltip component
  2. define a span element and apply the brnTooltip structural directive on it
  3. apply hlmTooltipTrigger to the button

Very complex, deeply nested and you need to know exactly which type of directive (hlm vs brn) to apply where.

Proposal

I know I'm gonna sound like a stuck CD on repeat, but Material's API is much more comfortable in this regard. Simply apply the [matTooltip] directive to the button and it's done.

Obviously the HlmTooltip allows for defining any template, also icons, for the tooltip, which is something I like and would definitely want to keep.

But for the majority of cases where I just want to apply a quick tooltip to a icon button (hence no icon in the tooltip needed) for which, I think, Material's approach is superior.

Describe any alternatives/workarounds you're currently using

No response

I would be willing to submit a PR to fix this issue

goetzrobin commented 3 months ago

@alexciesielski maybe we can support both? I'd like to keep the template way to support something like adding a button. But I also see that it's a lot easier to simply pass in a string and call it a day

alexciesielski commented 3 months ago

Yes, that is what I'm suggesting.

Keep the existing functionality but add a way to simply pass in a string and call it a day (like matTooltip does it)

alexciesielski commented 3 months ago

So can I go ahead with this? Or will it end up like the table PR? 😅

goetzrobin commented 3 months ago

No you can go ahead and add this. Still owe you a review/decision with that. Haven't had much toke to spend on spartan lately, unfortunately.

alexciesielski commented 3 months ago

Would you have anything against renaming hlmTooltipTrigger to just hlmTooltip? Similar to matTooltip? It looks weird to me to pass a string to the trigger, and it would be aligned with how Material does it, too.

image

alexciesielski commented 3 months ago

Also I'm not sure if we even need to wrap the whole thing inside a hlm-tooltip component for when using a TemplateRef? It should be possible to just pass it into the directive and have it handle everything? Or is there something I am missing for what the wrapping component does?

My proposal would be to just have a single brn/hlmTooltip directive, that takes both a string and a TemplateRef and renders them, without having to wrap the whole thing in another hlm-tooltip component.

goetzrobin commented 3 months ago

We are taking this approach to mimic the RadixUI behavior. Personally, I also prefer it because it let's me avoid having an ng-template with a ref to it, which I pass to the tooltip. I am open to supporting both, but want to keep the hlm-tooltip for now to stay consistent with this approach we used in other components.