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

USWDS - Tooltip: dynamic initialization #4955

Merged
merged 8 commits into from
Sep 26, 2022
Merged

USWDS - Tooltip: dynamic initialization #4955

merged 8 commits into from
Sep 26, 2022

Conversation

mejiaj
Copy link
Contributor

@mejiaj mejiaj commented Sep 20, 2022

Summary

Tooltip initializes on first interaction. Tooltip now checks if it has been initialized before toggling. If it hasn't initialized properly then it will call a setup attributes function. Closes #4055.

Additional information

Component setup only occurred on document ready or when manually calling the on/off method on a specific element. This caused issues because:

  1. Dynamically created components never initialized properly
  2. Event listeners weren't attached properly

This PR does the following:

  1. Init only scaffolds component e0a3cb4
  2. Use behaviors to add event listeners that bubble eb4c4d0#diff-8d7e7a60d39759da3b847a457caf221ad9251a8e84ab2356905ac7e9ba36cf4cR372-R383
  3. Scaffold component on first interaction (if it hasn't already  — helps with dynamically added components) 92d15e3
  4. Exports methods users can hook into (gives users more control over functionality) 585d2b3
  5. Updates JS entrypoint for Storybook  — avoids building JS every change, but still requires refresh

I've also removed the title attribute from tooltip trigger, instead of leaving it empty.

How to test

  • Confirm tooltip still functions as expected
  • Create a tooltip in JS (added mine via devtools console) and insert into DOM ★
  • Confirm tooltip works as expected

★ Dynamic tooltip example

// Create trigger based on recommended markup from Tooltip page
const dynamicTooltip = document.createElement("button");

// Add attributes
dynamicTooltip.setAttribute("type", "button");
dynamicTooltip.setAttribute("data-position", "right");
dynamicTooltip.setAttribute("title", "A dynamic tooltip");
dynamicTooltip.classList.add("usa-button", "usa-tooltip");

// Let's give our tooltip trigger some text
dynamicTooltip.textContent = "My dynamic tooltip";

// Insert into DOM
document.body.appendChild(dynamicTooltip);

Before you hit Submit, make sure you’ve done whichever of these applies to you:

  • Follow the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run npm test and make sure the tests for the files you have changed have passed.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.
  • Title your pull request using this format: [Website] - [UI component]: Brief statement describing what this pull request solves.

- Create const for tooltip trigger class
- Create function for getting tooltip elements
- Show and hide default tooltips using behaviors
- Remove title attribute on trigger instead of leaving it empty
Switched to using events in behaviors like other components. Export functions for setup, getting selectors, showing and hiding tooltip.
@mejiaj mejiaj linked an issue Sep 20, 2022 that may be closed by this pull request
@mejiaj mejiaj marked this pull request as ready for review September 21, 2022 17:03
@mejiaj mejiaj requested a review from amyleadem September 21, 2022 17:03
Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

Very cool fix! I was able to dynamically add a tooltip that scaffolds itself and works as expected. Additionally, the tooltip works as expected with both mouse and keyboard in Safari, Firefox, Chrome and with touch in iOS and VoiceOver Gestures.

@mejiaj mejiaj requested a review from thisisdano September 23, 2022 17:06
Copy link
Member

@thisisdano thisisdano left a comment

Choose a reason for hiding this comment

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

Such a good improvement and a model for other components!

@thisisdano thisisdano merged commit 96794bd into develop Sep 26, 2022
@thisisdano thisisdano deleted the jm-tooltip-init branch September 26, 2022 15:48
@jakerella
Copy link

❤️ THANK YOU!

This was referenced Oct 19, 2022
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.

Allow for dynamic initialization of tooltip component
4 participants