Skip to content

Commit

Permalink
[User Education] Add auto-dismiss timeout to WebUI IPH
Browse files Browse the repository at this point in the history
The HelpBubble will apply the given timeout or fallback to the default values. Default timeout logic is copied from Views IPH (with buttons = never, without buttons = 10s). Timeout gets to javascript as a BigInt and needs to be cast to Number. Timing out will now call HelpBubbleClosed in the HelpBubbleHandler.

Mojo changes:
- Pass TimeDelta through mojo for timeout
- Add HelpBubbleClosedReason enum
- Update HelpBubbleClosed to accept HelpBubbleClosedReason


Bug: 1350594
Change-Id: I7b6ae1802be5d44127e972dd5fe83859dbfc22e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3838844
Auto-Submit: Mickey Burks <mickeyburks@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Reviewed-by: Dana Fried <dfried@chromium.org>
Commit-Queue: Mickey Burks <mickeyburks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1042190}
  • Loading branch information
Mickey Burks authored and Chromium LUCI CQ committed Sep 1, 2022
1 parent 2d80b7d commit 4cee94e
Show file tree
Hide file tree
Showing 10 changed files with 212 additions and 32 deletions.
59 changes: 54 additions & 5 deletions chrome/test/data/webui/cr_components/help_bubble_mixin_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import 'chrome://webui-test/mojo_webui_test_support.js';
import 'chrome://resources/cr_components/help_bubble/help_bubble.js';

import {HelpBubbleElement} from 'chrome://resources/cr_components/help_bubble/help_bubble.js';
import {HelpBubbleArrowPosition, HelpBubbleClientCallbackRouter, HelpBubbleClientRemote, HelpBubbleHandlerInterface, HelpBubbleParams} from 'chrome://resources/cr_components/help_bubble/help_bubble.mojom-webui.js';
import {HelpBubbleArrowPosition, HelpBubbleClientCallbackRouter, HelpBubbleClientRemote, HelpBubbleClosedReason, HelpBubbleHandlerInterface, HelpBubbleParams} from 'chrome://resources/cr_components/help_bubble/help_bubble.mojom-webui.js';
import {HelpBubbleMixin, HelpBubbleMixinInterface} from 'chrome://resources/cr_components/help_bubble/help_bubble_mixin.js';
import {HelpBubbleProxy, HelpBubbleProxyImpl} from 'chrome://resources/cr_components/help_bubble/help_bubble_proxy.js';
import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
Expand Down Expand Up @@ -93,8 +93,8 @@ class TestHelpBubbleHandler extends TestBrowserProxy implements
this.methodCalled('helpBubbleButtonPressed', nativeIdentifier, button);
}

helpBubbleClosed(nativeIdentifier: string, byUser: boolean) {
this.methodCalled('helpBubbleClosed', nativeIdentifier, byUser);
helpBubbleClosed(nativeIdentifier: string, reason: HelpBubbleClosedReason) {
this.methodCalled('helpBubbleClosed', nativeIdentifier, reason);
}
}

Expand Down Expand Up @@ -141,6 +141,15 @@ suite('CrComponentsHelpBubbleMixinTest', () => {
return waitAfterNextRender(container);
}

/**
* Create a promise that resolves after a given amount of time
*/
async function sleep(milliseconds: number) {
return new Promise((res) => {
setTimeout(res, milliseconds);
});
}

setup(() => {
testProxy = new TestHelpBubbleProxy();
HelpBubbleProxyImpl.setInstance(testProxy);
Expand Down Expand Up @@ -334,7 +343,7 @@ suite('CrComponentsHelpBubbleMixinTest', () => {
assertEquals(
1, testProxy.getHandler().getCallCount('helpBubbleClosed'));
assertDeepEquals(
[[PARAGRAPH_NATIVE_ID, false]],
[[PARAGRAPH_NATIVE_ID, HelpBubbleClosedReason.kPageChanged]],
testProxy.getHandler().getArgs('helpBubbleClosed'));
assertFalse(container.isHelpBubbleShowing());
});
Expand All @@ -352,6 +361,19 @@ suite('CrComponentsHelpBubbleMixinTest', () => {
assertTrue(container.isHelpBubbleShowing());
});

test('help bubble mixin does not timeout by default', async () => {
container.showHelpBubble('p1', defaultParams);

// This is not the current bubble anchor, so should not send an event.
container.$.title.style.display = 'none';
await waitForVisibilityEvents();
assertEquals(0, testProxy.getHandler().getCallCount('helpBubbleClosed'));
assertTrue(container.isHelpBubbleShowing());
await sleep(100); // 100ms
assertEquals(0, testProxy.getHandler().getCallCount('helpBubbleClosed'));
assertTrue(container.isHelpBubbleShowing());
});

test('help bubble mixin reshow bubble', async () => {
testProxy.getCallbackRouterRemote().showHelpBubble(defaultParams);
await waitAfterNextRender(container);
Expand Down Expand Up @@ -463,7 +485,7 @@ suite('CrComponentsHelpBubbleMixinTest', () => {
await waitForVisibilityEvents();
assertEquals(1, testProxy.getHandler().getCallCount('helpBubbleClosed'));
assertDeepEquals(
[[PARAGRAPH_NATIVE_ID, true]],
[[PARAGRAPH_NATIVE_ID, HelpBubbleClosedReason.kDismissedByUser]],
testProxy.getHandler().getArgs('helpBubbleClosed'));
assertFalse(container.isHelpBubbleShowing());
});
Expand Down Expand Up @@ -503,4 +525,31 @@ suite('CrComponentsHelpBubbleMixinTest', () => {
testProxy.getHandler().getArgs('helpBubbleButtonPressed'));
assertFalse(container.isHelpBubbleShowing());
});

const timeoutParams: HelpBubbleParams = new HelpBubbleParams();
timeoutParams.nativeIdentifier = PARAGRAPH_NATIVE_ID;
timeoutParams.closeButtonAltText = CLOSE_BUTTON_ALT_TEXT;
timeoutParams.position = HelpBubbleArrowPosition.TOP_CENTER;
timeoutParams.bodyText = 'This is another help bubble.';
timeoutParams.titleText = 'This is a title';
timeoutParams.timeout = {
microseconds: BigInt(50 * 1000), // 50ms
};
timeoutParams.buttons = [];

test('help bubble mixin sends timeout event', async () => {
container.showHelpBubble('p1', timeoutParams);
await waitAfterNextRender(container);
assertEquals(
0, testProxy.getHandler().getCallCount('helpBubbleClosed'),
'helpBubbleClosed was not called');
await sleep(100); // 100ms
assertEquals(
1, testProxy.getHandler().getCallCount('helpBubbleClosed'),
'helpBubbleClosed was called');
assertDeepEquals(
[[PARAGRAPH_NATIVE_ID, HelpBubbleClosedReason.kTimedOut]],
testProxy.getHandler().getArgs('helpBubbleClosed'));
assertFalse(container.isHelpBubbleShowing());
});
});
48 changes: 47 additions & 1 deletion chrome/test/data/webui/cr_components/help_bubble_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import 'chrome://webui-test/mojo_webui_test_support.js';
import 'chrome://resources/cr_components/help_bubble/help_bubble.js';

import {CrButtonElement} from '//resources/cr_elements/cr_button/cr_button.js';
import {HELP_BUBBLE_DISMISSED_EVENT, HelpBubbleDismissedEvent, HelpBubbleElement} from 'chrome://resources/cr_components/help_bubble/help_bubble.js';
import {HELP_BUBBLE_DISMISSED_EVENT, HELP_BUBBLE_TIMED_OUT_EVENT, HelpBubbleDismissedEvent, HelpBubbleElement, HelpBubbleTimedOutEvent} from 'chrome://resources/cr_components/help_bubble/help_bubble.js';
import {HelpBubbleArrowPosition, HelpBubbleButtonParams} from 'chrome://resources/cr_components/help_bubble/help_bubble.mojom-webui.js';
import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {isVisible, waitAfterNextRender} from 'chrome://webui-test/test_util.js';
Expand Down Expand Up @@ -52,6 +52,15 @@ suite('CrComponentsHelpBubbleTest', () => {
return mainEl;
}

/**
* Create a promise that resolves after a given amount of time
*/
async function sleep(milliseconds: number) {
return new Promise((resolve) => {
setTimeout(resolve, milliseconds);
});
}

setup(() => {
document.body.innerHTML = `
<div id='container'>
Expand Down Expand Up @@ -194,6 +203,43 @@ suite('CrComponentsHelpBubbleTest', () => {
assertEquals(1, clicked, 'close button should be clicked once');
});

test('help bubble timeout generates event', async () => {
let timedOut: number = 0;
const callback = (e: HelpBubbleTimedOutEvent) => {
assertEquals(
'title', e.detail.anchorId, 'timeout event anchorId should match');
++timedOut;
};
helpBubble.addEventListener(HELP_BUBBLE_TIMED_OUT_EVENT, callback);
helpBubble.anchorId = 'title';
helpBubble.position = HelpBubbleArrowPosition.TOP_CENTER;
helpBubble.bodyText = HELP_BUBBLE_BODY;
helpBubble.timeoutMs = 250; // 250ms
helpBubble.show();
assertEquals(0, timedOut, 'timeout should not be triggered');
await waitAfterNextRender(helpBubble);
await sleep(500); // 500ms
assertEquals(1, timedOut, 'timeout should only emit event once');
});

test('help bubble without timeout does not generate event', async () => {
let timedOut: number = 0;
const callback = (e: HelpBubbleTimedOutEvent) => {
assertEquals(
'title', e.detail.anchorId, 'timeout event anchorId should match');
++timedOut;
};
helpBubble.addEventListener(HELP_BUBBLE_TIMED_OUT_EVENT, callback);
helpBubble.anchorId = 'title';
helpBubble.position = HelpBubbleArrowPosition.TOP_CENTER;
helpBubble.bodyText = HELP_BUBBLE_BODY;
helpBubble.show();
assertEquals(0, timedOut, 'timeout should not be triggered');
await waitAfterNextRender(helpBubble);
await sleep(500); // 500ms
assertEquals(0, timedOut, 'timeout is never triggered');
});

test('help bubble adds one button', async () => {
helpBubble.anchorId = 'title';
helpBubble.position = HelpBubbleArrowPosition.TOP_CENTER;
Expand Down
4 changes: 4 additions & 0 deletions components/user_education/common/help_bubble_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@

namespace user_education {

// The amount of time the promo should stay onscreen.
constexpr base::TimeDelta kDefaultTimeoutWithoutButtons = base::Seconds(10);
constexpr base::TimeDelta kDefaultTimeoutWithButtons = base::Seconds(0);

// Mirrors most values of views::BubbleBorder::Arrow.
// All values except kNone show a visible arrow between the bubble and the
// anchor element.
Expand Down
4 changes: 0 additions & 4 deletions components/user_education/views/help_bubble_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,6 @@ namespace user_education {

namespace {

// The amount of time the promo should stay onscreen.
constexpr base::TimeDelta kDefaultTimeoutWithoutButtons = base::Seconds(10);
constexpr base::TimeDelta kDefaultTimeoutWithButtons = base::Seconds(0);

// Maximum width of the bubble. Longer strings will cause wrapping.
constexpr int kBubbleMaxWidthDip = 340;

Expand Down
38 changes: 29 additions & 9 deletions components/user_education/webui/help_bubble_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,12 @@ std::unique_ptr<HelpBubbleWebUI> HelpBubbleHandlerBase::CreateHelpBubble(
mojom_params->close_button_alt_text =
base::UTF16ToUTF8(data.params->close_button_alt_text);
mojom_params->force_close_button = data.params->force_close_button;
auto timeout = data.params->timeout.value_or(
data.params->buttons.empty() ? kDefaultTimeoutWithoutButtons
: kDefaultTimeoutWithButtons);
if (!timeout.is_zero()) {
mojom_params->timeout = timeout;
}
mojom_params->position = HelpBubbleArrowToPosition(data.params->arrow);
if (data.params->progress) {
mojom_params->progress = help_bubble::mojom::Progress::New();
Expand Down Expand Up @@ -195,7 +201,9 @@ void HelpBubbleHandlerBase::HelpBubbleAnchorVisibilityChanged(
// has additional code which executes after it. If that changes, the weak
// pointer can be moved closer to the top of this method.
auto weak_ptr = weak_ptr_factory_.GetWeakPtr();
HelpBubbleClosed(identifier_name, false);
HelpBubbleClosed(
identifier_name,
help_bubble::mojom::HelpBubbleClosedReason::kPageChanged);
if (!weak_ptr)
return;
}
Expand Down Expand Up @@ -244,8 +252,9 @@ void HelpBubbleHandlerBase::HelpBubbleButtonPressed(
data->closing = false;
}

void HelpBubbleHandlerBase::HelpBubbleClosed(const std::string& identifier_name,
bool by_user) {
void HelpBubbleHandlerBase::HelpBubbleClosed(
const std::string& identifier_name,
help_bubble::mojom::HelpBubbleClosedReason reason) {
ElementData* const data = GetDataByName(identifier_name);
if (!data)
return;
Expand All @@ -261,13 +270,24 @@ void HelpBubbleHandlerBase::HelpBubbleClosed(const std::string& identifier_name,
auto weak_ptr = weak_ptr_factory_.GetWeakPtr();
data->closing = true;

if (by_user) {
base::OnceClosure callback = std::move(data->params->dismiss_callback);
if (callback) {
std::move(callback).Run();
if (!weak_ptr)
return;
base::OnceClosure callback;
switch (reason) {
case help_bubble::mojom::HelpBubbleClosedReason::kDismissedByUser: {
callback = std::move(data->params->dismiss_callback);
break;
}
case help_bubble::mojom::HelpBubbleClosedReason::kTimedOut: {
callback = std::move(data->params->timeout_callback);
break;
}
case help_bubble::mojom::HelpBubbleClosedReason::kPageChanged:
break;
}

if (callback) {
std::move(callback).Run();
if (!weak_ptr)
return;
}

// This could also theoretically trigger callbacks.
Expand Down
4 changes: 3 additions & 1 deletion components/user_education/webui/help_bubble_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ class HelpBubbleHandlerBase : public help_bubble::mojom::HelpBubbleHandler {
bool visible) final;
void HelpBubbleButtonPressed(const std::string& identifier_name,
uint8_t button) final;
void HelpBubbleClosed(const std::string& identifier_name, bool by_user) final;
void HelpBubbleClosed(
const std::string& identifier_name,
help_bubble::mojom::HelpBubbleClosedReason reason) final;

ElementData* GetDataByName(const std::string& identifier_name,
ui::ElementIdentifier* found_identifier = nullptr);
Expand Down
14 changes: 10 additions & 4 deletions components/user_education/webui/help_bubble_handler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ class TestHelpBubbleHandler : public HelpBubbleHandlerBase {
MATCHER_P(MatchesHelpBubbleParams, expected, "") {
EXPECT_EQ(expected->body_text, arg->body_text);
EXPECT_EQ(expected->close_button_alt_text, arg->close_button_alt_text);
EXPECT_EQ(expected->force_close_button, arg->force_close_button);
EXPECT_EQ(expected->timeout, arg->timeout);
EXPECT_EQ(expected->native_identifier, arg->native_identifier);
EXPECT_EQ(expected->position, arg->position);
EXPECT_EQ(expected->title_text, arg->title_text);
Expand Down Expand Up @@ -225,6 +227,7 @@ TEST_F(HelpBubbleHandlerTest, ShowHelpBubble) {
expected->close_button_alt_text =
base::UTF16ToUTF8(params.close_button_alt_text);
expected->position = help_bubble::mojom::HelpBubbleArrowPosition::TOP_CENTER;
expected->timeout = base::Seconds(10);

EXPECT_CALL(test_handler_->mock(),
ShowHelpBubble(MatchesHelpBubbleParams(expected.get())));
Expand Down Expand Up @@ -373,7 +376,8 @@ TEST_F(HelpBubbleHandlerTest, HelpBubbleClosedWhenClosedRemotely) {
EXPECT_CALL_IN_SCOPE(
closed, Run,
handler()->HelpBubbleClosed(
kHelpBubbleHandlerTestElementIdentifier.GetName(), false));
kHelpBubbleHandlerTestElementIdentifier.GetName(),
help_bubble::mojom::HelpBubbleClosedReason::kPageChanged));
EXPECT_FALSE(help_bubble->is_open());
}

Expand Down Expand Up @@ -434,7 +438,8 @@ TEST_F(HelpBubbleHandlerTest, HelpBubbleClosedWhenClosedByUserCallsDismiss) {
EXPECT_CALL_IN_SCOPE(
dismissed, Run,
handler()->HelpBubbleClosed(
kHelpBubbleHandlerTestElementIdentifier.GetName(), true));
kHelpBubbleHandlerTestElementIdentifier.GetName(),
help_bubble::mojom::HelpBubbleClosedReason::kDismissedByUser));
EXPECT_FALSE(help_bubble->is_open());
}

Expand Down Expand Up @@ -554,8 +559,9 @@ TEST_F(HelpBubbleHandlerTest, ShowMultipleBubblesAndCloseOneViaCallback) {
EXPECT_TRUE(help_bubble2->is_open());

// Close one bubble without closing the other.
handler()->HelpBubbleClosed(kHelpBubbleHandlerTestElementIdentifier.GetName(),
false);
handler()->HelpBubbleClosed(
kHelpBubbleHandlerTestElementIdentifier.GetName(),
help_bubble::mojom::HelpBubbleClosedReason::kPageChanged);
EXPECT_FALSE(help_bubble->is_open());
EXPECT_TRUE(help_bubble2->is_open());

Expand Down
21 changes: 18 additions & 3 deletions ui/webui/resources/cr_components/help_bubble/help_bubble.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

module help_bubble.mojom;

import "mojo/public/mojom/base/time.mojom";

// Contains IPC calls that allow the HelpBubbleFactory system to connect to
// the HelpBubbleMixin and HelpBubbleElement component.
//
Expand Down Expand Up @@ -33,6 +35,16 @@ enum HelpBubbleArrowPosition {
RIGHT_BOTTOM,
};

// Reason for dismissing a help bubble
enum HelpBubbleClosedReason {
// page navigated away, anchor disappeared, etc.
kPageChanged,
// user dismissed via button or close-icon
kDismissedByUser,
// timeout reached
kTimedOut,
};

// Simplified version of user_education::HelpBubbleButtonParams.
struct HelpBubbleButtonParams {
string text;
Expand All @@ -59,6 +71,9 @@ struct HelpBubbleParams {
bool force_close_button = false;
Progress? progress;
array<HelpBubbleButtonParams> buttons;

// Auto-dismiss timeout as TimeDelta
mojo_base.mojom.TimeDelta? timeout;
};

// Used by the controller to bootstrap IPC. Any WebUIController can implement
Expand Down Expand Up @@ -94,9 +109,9 @@ interface HelpBubbleHandler {
HelpBubbleButtonPressed(string native_identifier, uint8 button_index);

// Called when the help bubble anchored to `native_identifier` is closed,
// either because the element it is associated with goes away, or because the
// user canceled it (e.g. pressed the [x] button).
HelpBubbleClosed(string native_identifier, bool by_user);
// either because the element it is associated with goes away, the user
// canceled it, or the timeout was reached (e.g. pressed the [x] button).
HelpBubbleClosed(string native_identifier, HelpBubbleClosedReason reason);
};

// Represents WebUI component that can display help bubbles. The implementing UI
Expand Down
Loading

0 comments on commit 4cee94e

Please sign in to comment.