Skip to content

Commit

Permalink
[User Education] Keep track of external bubble showing status
Browse files Browse the repository at this point in the history
A follow-up CL for scroll tracking logic will need to know whether
a bubble is externally displayed and the last-known anchor visibility
value

Refactor help bubble mixin and controller:

- Keep track of bubble visibility for external bubbles
- Keep track of anchor visibility
- Rename hasElement/getElement to hasBubble/getBubble for clarity
- Rename callsite variables to clarify values
- Move anchor highlighting logic to controller
- Update outdated documentation/comments

Bug: 1405258
Change-Id: I85c5d8ec856abb78bbf5812b42a428b4740cfec8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4188837
Reviewed-by: Dana Fried <dfried@chromium.org>
Commit-Queue: Mickey Burks <mickeyburks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096331}
  • Loading branch information
Mickey Burks authored and Chromium LUCI CQ committed Jan 24, 2023
1 parent 64e710d commit 2dbe5f5
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 78 deletions.
12 changes: 6 additions & 6 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,9 +6,9 @@ import 'chrome://webui-test/mojo_webui_test_support.js';
import 'chrome://resources/cr_components/help_bubble/help_bubble.js';

import {IronIconElement} from '//resources/polymer/v3_0/iron-icon/iron-icon.js';
import {ANCHOR_HIGHLIGHT_CLASS, HelpBubbleElement} from '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, HelpBubbleClosedReason, HelpBubbleHandlerInterface, HelpBubbleParams} from 'chrome://resources/cr_components/help_bubble/help_bubble.mojom-webui.js';
import {HelpBubbleController} from 'chrome://resources/cr_components/help_bubble/help_bubble_controller.js';
import {ANCHOR_HIGHLIGHT_CLASS, HelpBubbleController} from 'chrome://resources/cr_components/help_bubble/help_bubble_controller.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 @@ -281,10 +281,10 @@ suite('CrComponentsHelpBubbleMixinTest', () => {
test(
'help bubble mixin shows bubble anchored to arbitrary HTMLElment', () => {
assertFalse(container.isHelpBubbleShowing());
assertFalse(spanBubble.isShowing());
assertFalse(spanBubble.isBubbleShowing());
container.showHelpBubble(spanBubble, defaultParams);
assertTrue(container.isHelpBubbleShowing());
assertTrue(spanBubble.isShowing());
assertTrue(spanBubble.isBubbleShowing());
});

test(
Expand All @@ -308,10 +308,10 @@ suite('CrComponentsHelpBubbleMixinTest', () => {
'help bubble anchors to correct element in shadow dom');

assertFalse(container.isHelpBubbleShowing());
assertFalse(nestedChildBubble.isShowing());
assertFalse(nestedChildBubble.isBubbleShowing());
container.showHelpBubble(nestedChildBubble, defaultParams);
assertTrue(container.isHelpBubbleShowing());
assertTrue(nestedChildBubble.isShowing());
assertTrue(nestedChildBubble.isBubbleShowing());
});

test('help bubble mixin reports not open for other elements', () => {
Expand Down
15 changes: 0 additions & 15 deletions ui/webui/resources/cr_components/help_bubble/help_bubble.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ import {HelpBubbleArrowPosition, HelpBubbleButtonParams, Progress} from './help_

const ACTION_BUTTON_ID_PREFIX = 'action-button-';

export const ANCHOR_HIGHLIGHT_CLASS = 'help-anchor-highlight';

export const HELP_BUBBLE_DISMISSED_EVENT = 'help-bubble-dismissed';
export const HELP_BUBBLE_TIMED_OUT_EVENT = 'help-bubble-timed-out';

Expand Down Expand Up @@ -146,7 +144,6 @@ export class HelpBubbleElement extends PolymerElement {
this.style.display = 'block';
this.removeAttribute('aria-hidden');
this.updatePosition_();
this.setAnchorHighlight_(true);

this.debouncedUpdate = debounceEnd(() => {
if (this.anchorElement_) {
Expand Down Expand Up @@ -182,7 +179,6 @@ export class HelpBubbleElement extends PolymerElement {
hide() {
this.style.display = 'none';
this.setAttribute('aria-hidden', 'true');
this.setAnchorHighlight_(false);
this.anchorElement_ = null;
if (this.timeoutTimerId !== null) {
clearInterval(this.timeoutTimerId);
Expand Down Expand Up @@ -475,17 +471,6 @@ export class HelpBubbleElement extends PolymerElement {
this.style.top = offsetY.toString() + 'px';
this.style.left = offsetX.toString() + 'px';
}

/**
* Styles the anchor element to appear highlighted while the bubble is open,
* or removes the highlight.
*/
private setAnchorHighlight_(highlight: boolean) {
assert(
this.anchorElement_,
'Set anchor highlight: expected valid anchor element.');
this.anchorElement_.classList.toggle(ANCHOR_HIGHLIGHT_CLASS, highlight);
}
}

customElements.define(HelpBubbleElement.is, HelpBubbleElement);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ type Root = HTMLElement|ShadowRoot&{shadowRoot?: ShadowRoot};

export type Trackable = string|string[]|HTMLElement;

export const ANCHOR_HIGHLIGHT_CLASS = 'help-anchor-highlight';

/**
* HelpBubble controller class
* - There should exist only one HelpBubble instance for each nativeId
Expand All @@ -22,25 +24,47 @@ export class HelpBubbleController {
private nativeId_: string;
private root_: ShadowRoot;
private anchor_: HTMLElement|null = null;
private showing_: boolean = false;
private bubble_: HelpBubbleElement|null = null;
private padding_: InsetsF = new InsetsF();

/**
* Whether a help bubble (webui or external) is being shown for this
* controller
*/
private isBubbleShowing_: boolean = false;

// Keep track of last-known anchor visibility status
private isAnchorVisible_: boolean = false;

/*
* This flag is used to know whether to send position updates for
* external bubbles
*/
private isExternal_: boolean = false;

constructor(nativeId: string, root: ShadowRoot) {
assert(nativeId && root);

this.nativeId_ = nativeId;
this.root_ = root;
}

isShowing() {
return this.showing_;
isBubbleShowing() {
return this.isBubbleShowing_;
}

canShow() {
canShowBubble() {
return this.hasAnchor();
}

hasBubble() {
return !!this.bubble_;
}

getBubble() {
return this.bubble_;
}

hasAnchor() {
return !!this.anchor_;
}
Expand All @@ -57,6 +81,20 @@ export class HelpBubbleController {
return this.padding_;
}

getAnchorVisibility() {
return this.isAnchorVisible_;
}

cacheAnchorVisibility(isVisible: boolean) {
this.isAnchorVisible_ = isVisible;
}

updateExternalShowingStatus(isShowing: boolean) {
this.isExternal_ = true;
this.isBubbleShowing_ = isShowing;
this.setAnchorHighlight_(isShowing);
}

track(trackable: Trackable, padding: InsetsF): boolean {
assert(!this.anchor_);

Expand Down Expand Up @@ -97,21 +135,15 @@ export class HelpBubbleController {
return cur as HTMLElement;
}

hasElement() {
return !!this.bubble_;
}

getElement() {
return this.bubble_;
}

show() {
this.isExternal_ = false;
if (!(this.bubble_ && this.anchor_)) {
return;
}
this.bubble_.show(this.anchor_);
this.anchor_.focus();
this.showing_ = true;
this.isBubbleShowing_ = true;
this.setAnchorHighlight_(true);
}

hide() {
Expand All @@ -121,7 +153,8 @@ export class HelpBubbleController {
this.bubble_.hide();
this.bubble_.remove();
this.bubble_ = null;
this.showing_ = false;
this.isBubbleShowing_ = false;
this.setAnchorHighlight_(false);
}

createBubble(params: HelpBubbleParams): HelpBubbleElement {
Expand Down Expand Up @@ -154,4 +187,15 @@ export class HelpBubbleController {

return this.bubble_;
}


/**
* Styles the anchor element to appear highlighted while the bubble is open,
* or removes the highlight.
*/
private setAnchorHighlight_(highlight: boolean) {
assert(
this.anchor_, 'Set anchor highlight: expected valid anchor element.');
this.anchor_.classList.toggle(ANCHOR_HIGHLIGHT_CLASS, highlight);
}
}

0 comments on commit 2dbe5f5

Please sign in to comment.