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

Prevent default/stop propagation on Tooltip.disclosure click #1476

Merged
merged 9 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 41 additions & 23 deletions component-catalog/src/Examples/Tooltip.elm
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import Debug.Control.View as ControlView
import EllieLink
import Example exposing (Example)
import Html.Styled.Attributes exposing (css, href, id)
import Html.Styled.Events as Events
import KeyboardSupport exposing (Key(..))
import Markdown
import Nri.Ui.ClickableSvg.V2 as ClickableSvg
Expand Down Expand Up @@ -93,6 +94,7 @@ example =
type alias State =
{ openTooltip : Maybe TooltipId
, staticExampleSettings : Control (List ( String, Tooltip.Attribute Never ))
, disclosureModel : { parentClicks : Int }
, pageSettings : Control PageSettings
}

Expand All @@ -101,6 +103,7 @@ init : State
init =
{ openTooltip = Nothing
, staticExampleSettings = initStaticExampleSettings
, disclosureModel = { parentClicks = 0 }
, pageSettings =
Control.record PageSettings
|> Control.field "backgroundColor"
Expand Down Expand Up @@ -129,6 +132,11 @@ type Msg
| SetControl (Control (List ( String, Tooltip.Attribute Never )))
| UpdatePageSettings (Control PageSettings)
| Log String
| DisclosureMsg DisclosureMsg


type DisclosureMsg
= ParentClick


update : Msg -> State -> ( State, Cmd Msg )
Expand All @@ -150,6 +158,9 @@ update msg model =
Log message ->
( Debug.log "Tooltip Log:" |> always model, Cmd.none )

DisclosureMsg ParentClick ->
( { model | disclosureModel = { parentClicks = model.disclosureModel.parentClicks + 1 } }, Cmd.none )


view : EllieLink.Config -> State -> List (Html Msg)
view ellieLinkConfig model =
Expand Down Expand Up @@ -238,7 +249,7 @@ Sometimes a tooltip trigger doesn't have any functionality itself outside of rev

This behavior is analogous to disclosure behavior, except that it's presented different visually. (For more information, please read [Sarah Higley's "Tooltips in the time of WCAG 2.1" post](https://sarahmhigley.com/writing/tooltips-in-wcag-21).)
"""
, example = viewDisclosureToolip model.openTooltip
, example = viewDisclosureToolip model.openTooltip model.disclosureModel
, tooltipId = Disclosure
}
, { name = "Tooltip.viewToggleTip"
Expand Down Expand Up @@ -304,38 +315,45 @@ viewAuxillaryDescriptionToolip openTooltip =
]


viewDisclosureToolip : Maybe TooltipId -> Html Msg
viewDisclosureToolip openTooltip =
viewDisclosureToolip : Maybe TooltipId -> { parentClicks : Int } -> Html Msg
viewDisclosureToolip openTooltip { parentClicks } =
let
triggerId =
"tooltip__disclosure-trigger"

lastId =
"tooltip__disclosure-what-is-mastery"
in
Tooltip.view
{ id = "tooltip__disclosure"
, trigger =
\eventHandlers ->
ClickableSvg.button "Previously mastered"
(Svg.withColor Colors.green UiIcon.starFilled)
[ ClickableSvg.custom eventHandlers
, ClickableSvg.id triggerId
Html.button
[ css [ Css.padding (Css.px 40) ]
, Events.onClick (DisclosureMsg ParentClick)
, id "parent-button"
]
[ Tooltip.view
{ id = "tooltip__disclosure"
, trigger =
\eventHandlers ->
ClickableSvg.button "Previously mastered"
(Svg.withColor Colors.green UiIcon.starFilled)
[ ClickableSvg.custom eventHandlers
, ClickableSvg.id triggerId
]
}
[ Tooltip.html
[ Html.text "You mastered this skill in a previous year! Way to go! "
, Html.a
[ id lastId
, href "https://noredink.zendesk.com/hc/en-us/articles/203022319-What-is-mastery-"
]
}
[ Tooltip.html
[ Html.text "You mastered this skill in a previous year! Way to go! "
, Html.a
[ id lastId
, href "https://noredink.zendesk.com/hc/en-us/articles/203022319-What-is-mastery-"
[ Html.text "Learn more about NoRedInk Mastery" ]
]
[ Html.text "Learn more about NoRedInk Mastery" ]
, Tooltip.disclosure { triggerId = triggerId, lastId = Just lastId }
, Tooltip.onToggle (ToggleTooltip Disclosure)
, Tooltip.open (openTooltip == Just Disclosure)
, Tooltip.smallPadding
, Tooltip.alignEndForMobile (Css.px 148)
]
, Tooltip.disclosure { triggerId = triggerId, lastId = Just lastId }
, Tooltip.onToggle (ToggleTooltip Disclosure)
, Tooltip.open (openTooltip == Just Disclosure)
, Tooltip.smallPadding
, Tooltip.alignEndForMobile (Css.px 148)
, Html.div [ id "parent-button-clicks" ] [ Html.text ("Parent Clicks: " ++ String.fromInt parentClicks) ]
]


Expand Down
50 changes: 50 additions & 0 deletions script/puppeteer-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,59 @@ describe("UI tests", function () {
handleAxeResults(name, results);
};

const tooltipProcessing = async (name, location) => {
await defaultProcessing(name, location);
await page.waitForSelector("#parent-button-clicks");

const buttonClick = async () => {
const button = await page.$("#parent-button");
await page.evaluate((el) => el.click(), button);
await page.waitForTimeout(100);
};

const getCounterText = async () => {
const counter = await page.$("#parent-button-clicks");
const text = await page.evaluate((el) => el.innerText, counter);
return text;
};

const tooltipTriggerClick = async () => {
const button = await page.$("#tooltip__disclosure-trigger");
await page.evaluate((el) => el.click(), button);
await page.waitForTimeout(100);
};

const isTooltipVisible = async () => {
let res = await page.evaluate(() => {
return (
getComputedStyle(
document.getElementById("tooltip__disclosure").parentElement
).getPropertyValue("display") != "none"
);
});

return res;
};

assert.equal(await getCounterText(), "Parent Clicks: 0");
assert.equal(await isTooltipVisible(), false);
await tooltipTriggerClick();
assert.equal(await isTooltipVisible(), true);
await tooltipTriggerClick();
assert.equal(await isTooltipVisible(), false);
assert.equal(await getCounterText(), "Parent Clicks: 0");
await buttonClick();
assert.equal(await getCounterText(), "Parent Clicks: 1");
await buttonClick();
assert.equal(await getCounterText(), "Parent Clicks: 2");
};

const skippedRules = {
// Loading's color contrast check seems to change behavior depending on whether Percy snapshots are taken or not
Loading: ["color-contrast"],
RadioButton: ["duplicate-id"],
// We need nested-interactive to test the tooltip behavior
Tooltip: ["nested-interactive"],
};

const specialProcessing = {
Expand All @@ -184,6 +233,7 @@ describe("UI tests", function () {
UiIcon: iconProcessing,
Logo: iconProcessing,
Pennant: iconProcessing,
Tooltip: tooltipProcessing,
};

it("All", async function () {
Expand Down
4 changes: 3 additions & 1 deletion src/Nri/Ui/Tooltip/V3.elm
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ module Nri.Ui.Tooltip.V3 exposing
- adds `paragraph` and `markdown` support
- add partially-transparent white border around tooltips
- Use Nri.Ui.WhenFocusLeaves.V2
- prevent default and stop propagation on click for disclosure tooltips

Changes from V2:

Expand Down Expand Up @@ -87,6 +88,7 @@ import Content
import Css exposing (Color, Px, Style)
import Css.Global as Global
import Css.Media
import EventExtras as Events
import Html.Styled as Root
import Html.Styled.Attributes as Attributes
import Html.Styled.Events as Events
Expand Down Expand Up @@ -940,7 +942,7 @@ viewTooltip_ { trigger, id } tooltip =
, tabForwardAction = msg False
}
]
, [ Events.onClick (msg (not tooltip.isOpen))
, [ Events.onClickPreventDefaultAndStopPropagation (msg (not tooltip.isOpen))
, Key.onKeyDown [ Key.escape (msg False) ]
]
)
Expand Down
47 changes: 45 additions & 2 deletions tests/Spec/Nri/Ui/Tooltip.elm
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
module Spec.Nri.Ui.Tooltip exposing (spec)

import Accessibility.Aria as Aria
import Html.Attributes as Attributes
import Expect
import Html.Attributes
import Html.Styled as HtmlStyled
import Nri.Ui.ClickableText.V3 as ClickableText
import Nri.Ui.Tooltip.V3 as Tooltip
import ProgramTest exposing (ProgramTest, ensureViewHas, ensureViewHasNot)
import Spec.Helpers exposing (nriDescription)
Expand Down Expand Up @@ -82,6 +84,47 @@ spec =
]
|> ProgramTest.ensureViewHasNot (id tooltipId :: tooltipContentSelector tooltipContent)
|> ProgramTest.done
, test "Prevents default on disclosures" <|
\() ->
let
tooltipContent =
"This will be the primary label"

triggerContent =
"label-less icon"

tooltipId =
"primary-label"

triggerId =
"trigger"

view model =
Tooltip.view
{ trigger =
\attributes ->
ClickableText.button triggerContent
[ ClickableText.custom attributes
, ClickableText.id triggerId
]
, id = tooltipId
}
[ Tooltip.open model.isOpen
, Tooltip.plaintext tooltipContent
, Tooltip.primaryLabel
, Tooltip.onToggle (\_ -> ())
, Tooltip.disclosure
{ triggerId = triggerId
, lastId = Nothing
}
]
|> HtmlStyled.toUnstyled
in
view { isOpen = False }
|> Query.fromHtml
|> Query.find [ Selector.id triggerId ]
|> Event.simulate Event.click
|> Expect.all [ Event.expectStopPropagation, Event.expectPreventDefault ]
]


Expand All @@ -102,7 +145,7 @@ program view attributes =

tooltipContentSelector : String -> List Selector.Selector
tooltipContentSelector tooltipContent =
[ Selector.attribute (Attributes.attribute "data-tooltip-visible" "true")
[ Selector.attribute (Html.Attributes.attribute "data-tooltip-visible" "true")
, Selector.containing [ text tooltipContent ]
]

Expand Down
Loading