Skip to content

Commit

Permalink
Polymer: Replace --paper-tooltip CSS Mixin with a 'tooltip' CSS Part.
Browse files Browse the repository at this point in the history
CSS Mixins are discouraged (only work with a polyfil anyway), and CSS
Shadow Parts are a native and standardized replacement for them.
Removing the --paper-tooltip mixin by locally patching
third_party/polymer/ to use CSS Shadow Parts instead.

Using CSS Parts avoids a complication in PrivacySandbox due to how this
UI is built side by side with Settings, which confuses the
polymer-css-build tool (invoked as part of optimize_webui()). Removing
previous workaround for dealing with this situation.

Also moving local modifications to paper-tooltip to their own dedicated
patch file, which makes it a bit easier to update local patches to
Polymer elements, compared to having everything hosted in a single chromium.patch file.

Fixed: 1308262
Change-Id: Ibbc8383c530ea3e09982a868cc786354cab006ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3642611
Reviewed-by: John Lee <johntlee@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002703}
  • Loading branch information
freshp86 authored and Chromium LUCI CQ committed May 12, 2022
1 parent 90e6f39 commit 7e9c69c
Show file tree
Hide file tree
Showing 12 changed files with 169 additions and 185 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,15 @@
width: 100%;
}

#albumTitleTooltip::part(tooltip) {
align-items: flex-end;
display: flex;
flex-direction: row;
height: 18px;
padding: 3px 8px;
}

#albumTitleTooltip {
--paper-tooltip: {
align-items: flex-end;
display: flex;
flex-direction: row;
height: 18px;
padding: 3px 8px;
};
--paper-tooltip-background: var(--cros-tooltip-background-color);
--paper-tooltip-opacity: 0.8;
--paper-tooltip-text-color: var(--cros-tooltip-label-color);
Expand Down
20 changes: 8 additions & 12 deletions chrome/browser/resources/chromeos/emoji_picker/emoji_button.html
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,15 @@
--paper-tooltip-delay-out: var(--emoji-tooltip-delay-out);
--paper-tooltip-duration-in: 0;
--paper-tooltip-duration-out: 0;
}

/* CSS mixin goes against style guide, but this is the only way to style
* the paper tooltip without changing the source code (which is in
* third_party)
*/
--paper-tooltip: {
box-shadow: var(--cr-elevation-1);
font-family: 'Roboto', sans-serif;
font-size: 12px;
margin: 4px;
padding: 4px 8px 4px 8px;
white-space: nowrap;
};
#tooltip::part(tooltip) {
box-shadow: var(--cr-elevation-1);
font-family: 'Roboto', sans-serif;
font-size: 12px;
margin: 4px;
padding: 4px 8px 4px 8px;
white-space: nowrap;
}
</style>

Expand Down
22 changes: 9 additions & 13 deletions chrome/browser/resources/chromeos/emoji_picker/emoticon_group.html
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,15 @@
--paper-tooltip-duration-out: 0;
--paper-tooltip-opacity: 1;
--paper-tooltip-text-color: var(--cros-tooltip-label-color);
}

/* CSS mixin goes against style guide, but this is the only way to style
* the paper tooltip without changing the source code (which is in
* third_party)
*/
--paper-tooltip: {
box-shadow: var(--cr-elevation-1);
font-family: 'Roboto', sans-serif;
font-size: 12px;
margin: 4px;
padding: 4px 8px 4px 8px;
white-space: nowrap;
};
.tooltip::part(tooltip) {
box-shadow: var(--cr-elevation-1);
font-family: 'Roboto', sans-serif;
font-size: 12px;
margin: 4px;
padding: 4px 8px 4px 8px;
white-space: nowrap;
}
</style>

Expand Down Expand Up @@ -165,4 +161,4 @@
[[emoticon.base.name]]
</paper-tooltip>
</template>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@
--paper-tooltip-background: var(--cros-tooltip-background-color);
--paper-tooltip-opacity: 1;
--paper-tooltip-text-color: var(--cros-tooltip-label-color);
--paper-tooltip: {
margin-inline-end: 6px;
};
}

#dataUsageDataTooltip::part(tooltip) {
margin-inline-end: 6px;
}

#dataUsageDataTooltipText {
Expand Down
10 changes: 10 additions & 0 deletions chrome/browser/resources/settings/privacy_sandbox/app.html
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,16 @@
--paper-tooltip-duration-in: 300;
--paper-tooltip-opacity: 1;
}

paper-tooltip::part(tooltip) {
border-radius: 4px;
box-shadow: var(--cr-elevation-2);
font-family: Roboto, Arial, sans-serif;
font-size: inherit;
font-weight: 400;
line-height: 154%; /* 20px. */
margin: 0 4px;
}
</style>

<settings-prefs id="prefs" prefs="{{prefs}}"></settings-prefs>
Expand Down
24 changes: 0 additions & 24 deletions chrome/browser/resources/settings/privacy_sandbox/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,30 +390,6 @@ export class PrivacySandboxAppElement extends PrivacySandboxAppElementBase {
target.id === 'topicsTooltipIcon' ? '#topicsTooltip' :
'#fledgeTooltip')!;

// Directly inject the required style into the stylesheets of the paper
// tooltip element. This is a workaround for CSS mixin properties seemingly
// being removed in optimized WebUI builds, and the paper-tooltip not
// supporting other styling methods.
// TODO(crbug.com/1308262): Expose required style hooks on paper-tooltip
const sheet = new CSSStyleSheet();
sheet.replaceSync(`
#tooltip {
border-radius: 4px;
box-shadow: var(--cr-elevation-2);
font-family: Roboto, Arial, sans-serif;
font-size: inherit;
font-weight: 400;
line-height: 154%; /* 20px. */
margin: 0 4px;
}`);
const elemStyleSheets = tooltip.shadowRoot!.adoptedStyleSheets;

if (elemStyleSheets.length === 0 ||
JSON.stringify(elemStyleSheets.slice(-1)[0]) !==
JSON.stringify(sheet)) {
tooltip.shadowRoot!.adoptedStyleSheets = [...elemStyleSheets, sheet];
}

const hide = () => {
tooltip.hide();
target.removeEventListener('mouseleave', hide);
Expand Down
108 changes: 0 additions & 108 deletions third_party/polymer/v3_0/chromium.patch
Original file line number Diff line number Diff line change
Expand Up @@ -171,45 +171,6 @@ index 99768f419688..7dd55b8077b7 100644

/**
* @struct
diff --git a/components-chromium/paper-tooltip/paper-tooltip.js b/components-chromium/paper-tooltip/paper-tooltip.js
index 853eee199025..303d1bbdfc78 100644
--- a/components-chromium/paper-tooltip/paper-tooltip.js
+++ b/components-chromium/paper-tooltip/paper-tooltip.js
@@ -311,12 +311,16 @@ Polymer({

/**
* Returns the target element that this tooltip is anchored to. It is
- * either the element given by the `for` attribute, or the immediate parent
- * of the tooltip.
+ * either the element given by the `for` attribute, the element manually
+ * specified through the `target` attribute, or the immediate parent of
+ * the tooltip.
*
* @type {Node}
*/
get target() {
+ if (this._manualTarget)
+ return this._manualTarget;
+
var parentNode = dom(this).parentNode;
// If the parentNode is a document fragment, then we need to use the host.
var ownerRoot = dom(this).getOwnerRoot();
@@ -331,6 +335,15 @@ Polymer({
return target;
},

+ /**
+ * Sets the target element that this tooltip will be anchored to.
+ * @param {Node} target
+ */
+ set target(target) {
+ this._manualTarget = target;
+ this._findTarget();
+ },
+
/**
* @return {void}
*/
diff --git a/components-chromium/paper-styles/color.js b/components-chromium/paper-styles/color.js
index 6af2fa359336..c56a6737e1d8 100644
--- a/components-chromium/paper-styles/color.js
Expand Down Expand Up @@ -433,72 +394,3 @@ index 6af2fa359336..c56a6737e1d8 100644
--paper-grey-50: #fafafa;
--paper-grey-100: #f5f5f5;
--paper-grey-200: #eeeeee;
diff --git a/components-chromium/paper-tooltip/paper-tooltip.js b/components-chromium/paper-tooltip/paper-tooltip.js
index 303d1bbdfc787..4adc8ea38d760 100644
--- a/components-chromium/paper-tooltip/paper-tooltip.js
+++ b/components-chromium/paper-tooltip/paper-tooltip.js
@@ -442,13 +442,16 @@ Polymer({
* @return {void}
*/
updatePosition: function() {
- if (!this._target || !this.offsetParent)
+ if (!this._target)
+ return;
+ var offsetParent = this._composedOffsetParent();
+ if (!offsetParent)
return;
var offset = this.offset;
// If a marginTop has been provided by the user (pre 1.0.3), use it.
if (this.marginTop != 14 && this.offset == 14)
offset = this.marginTop;
- var parentRect = this.offsetParent.getBoundingClientRect();
+ var parentRect = offsetParent.getBoundingClientRect();
var targetRect = this._target.getBoundingClientRect();
var thisRect = this.getBoundingClientRect();
var horizontalCenterOffset = (targetRect.width - thisRect.width) / 2;
@@ -594,5 +597,45 @@ Polymer({
}
this.unlisten(this.$.tooltip, 'animationend', '_onAnimationEnd');
this.unlisten(this, 'mouseenter', 'hide');
+ },
+
+ /**
+ * Polyfills the old offsetParent behavior from before the spec was changed:
+ * https://github.com/w3c/csswg-drafts/issues/159
+ */
+ _composedOffsetParent: function() {
+ // Do an initial walk to check for display:none ancestors.
+ for (let ancestor = this; ancestor; ancestor = flatTreeParent(ancestor)) {
+ if (!(ancestor instanceof Element))
+ continue;
+ if (getComputedStyle(ancestor).display === 'none')
+ return null;
+ }
+
+ for (let ancestor = flatTreeParent(this); ancestor; ancestor = flatTreeParent(ancestor)) {
+ if (!(ancestor instanceof Element))
+ continue;
+ const style = getComputedStyle(ancestor);
+ if (style.display === 'contents') {
+ // display:contents nodes aren't in the layout tree so they should be skipped.
+ continue;
+ }
+ if (style.position !== 'static') {
+ return ancestor;
+ }
+ if (ancestor.tagName === 'BODY')
+ return ancestor;
+ }
+ return null;
+
+ function flatTreeParent(element) {
+ if (element.assignedSlot) {
+ return element.assignedSlot;
+ }
+ if (element.parentNode instanceof ShadowRoot) {
+ return element.parentNode.host;
+ }
+ return element.parentNode;
+ }
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ Polymer({
color: var(--paper-tooltip-text-color, white);
padding: 8px;
border-radius: 2px;
@apply --paper-tooltip;
}
@keyframes keyFrameScaleUp {
Expand Down Expand Up @@ -222,7 +221,7 @@ Polymer({
}
</style>
<div id="tooltip" class="hidden">
<div id="tooltip" class="hidden" part="tooltip">
<slot></slot>
</div>
`,
Expand Down

0 comments on commit 7e9c69c

Please sign in to comment.