Skip to content

USWDS - Tooltip: Create a mixin for the tooltip spacer#5933

Merged
amyleadem merged 3 commits into
al-tooltip-hoverablefrom
jm-tooltip-hoverable
May 20, 2024
Merged

USWDS - Tooltip: Create a mixin for the tooltip spacer#5933
amyleadem merged 3 commits into
al-tooltip-hoverablefrom
jm-tooltip-hoverable

Conversation

@mejiaj

@mejiaj mejiaj commented May 20, 2024

Copy link
Copy Markdown
Contributor

Summary

Created mixin for tooltip spacer to avoid duplicate code.

Breaking change

This is not a breaking change.

Related issue

#5918

Related pull requests

Related to #5919.

Preview link

Preview link: Tooltip →

Problem statement

The CSS for creating the tooltip spacer was repetitive.

Solution

Created a mixin to manage this in one place.

Testing and review

Tooltips should create a spacer element between trigger and body that keeps the content visible.

@mejiaj mejiaj requested review from amyleadem and mahoneycm May 20, 2024 15:07
/// Create a spacer to increase target area for tooltip triangle.
///
/// @param {String} $direction - The direction of the tooltip; can be top, bottom, left, right.
/// @param {Length} $triangle-size [5px] - The size of the triangle.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it correct to define $triangle-size as a parameter here even though it's not being passed into the mixin?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. I don't think it's necessary since triangle size is already customizable.

I've moved the variable above the mixin in 04dcd6b to make it clearer that this mixin relies on that.

@mejiaj mejiaj requested a review from mahoneycm May 20, 2024 15:21

@amyleadem amyleadem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for taking this on @mejiaj! I confirmed that this mixin positions the spacers correctly (See screenshot below. Note: spacers are highlighted in orange only for demonstration purposes). The code looks great and is easy to understand.

image

The `$triangle-size` variable isn't a parameter, so removed from SASSDoc comment.

@mahoneycm mahoneycm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Working like a charm!

@amyleadem amyleadem merged commit babcc2a into al-tooltip-hoverable May 20, 2024
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.

3 participants