Skip to content

Commit

Permalink
Rename pop-up hover delay CSS property names
Browse files Browse the repository at this point in the history
See discussion at [1], but these seem like better names. This
CL changes:

  hover-pop-up-delay      -> pop-up-show-delay
  hover-pop-up-hide-delay -> pop-up-hide-delay

This is a mostly mechanical naming change.

[1] openui/open-ui#526 (comment)

Bug: 1307772
Change-Id: I96e6b7cb88821a30ebc05678276cd53b13616ec3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3811136
Reviewed-by: Joey Arhar <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1032057}
NOKEYCHECK=True
GitOrigin-RevId: 016a16ebf65e716286cc7be0d4b17f49589616b4
  • Loading branch information
mfreed7 authored and copybara-github committed Aug 5, 2022
1 parent 6aa3edd commit 2b4b30f
Show file tree
Hide file tree
Showing 15 changed files with 58 additions and 58 deletions.
4 changes: 2 additions & 2 deletions blink/public/mojom/use_counter/metrics/css_property_id.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -772,8 +772,8 @@ enum CSSSampleId {
kAnchorName = 719,
kPositionFallback = 720,
kAnchorScroll = 721,
kHoverPopUpDelay = 722,
kHoverPopUpHideDelay = 723,
kPopUpShowDelay = 722,
kPopUpHideDelay = 723,
// 1. Add new features above this line (don't change the assigned numbers of
// the existing items).
// 2. Run the src/tools/metrics/histograms/update_use_counter_css.py script
Expand Down
4 changes: 2 additions & 2 deletions blink/renderer/core/animation/css_interpolation_types_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,8 @@ const InterpolationTypes& CSSInterpolationTypesMap::Get(
applicable_types->push_back(
std::make_unique<CSSNumberInterpolationType>(used_property));
break;
case CSSPropertyID::kHoverPopUpDelay:
case CSSPropertyID::kHoverPopUpHideDelay:
case CSSPropertyID::kPopUpShowDelay:
case CSSPropertyID::kPopUpHideDelay:
applicable_types->push_back(
std::make_unique<CSSTimeInterpolationType>(used_property));
break;
Expand Down
20 changes: 10 additions & 10 deletions blink/renderer/core/animation/css_time_interpolation_type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ absl::optional<double> CSSTimeInterpolationType::GetSeconds(
const CSSPropertyID& property,
const ComputedStyle& style) {
switch (property) {
case CSSPropertyID::kHoverPopUpDelay:
return style.HoverPopUpDelay();
case CSSPropertyID::kHoverPopUpHideDelay:
return style.HoverPopUpHideDelay();
case CSSPropertyID::kPopUpShowDelay:
return style.PopUpShowDelay();
case CSSPropertyID::kPopUpHideDelay:
return style.PopUpHideDelay();
default:
NOTREACHED();
return absl::optional<double>();
Expand All @@ -70,8 +70,8 @@ absl::optional<double> CSSTimeInterpolationType::GetSeconds(
double CSSTimeInterpolationType::ClampTime(const CSSPropertyID& property,
double value) const {
switch (property) {
case CSSPropertyID::kHoverPopUpDelay:
case CSSPropertyID::kHoverPopUpHideDelay:
case CSSPropertyID::kPopUpShowDelay:
case CSSPropertyID::kPopUpHideDelay:
return ClampTo<float>(value, 0);
default:
NOTREACHED();
Expand All @@ -96,11 +96,11 @@ void CSSTimeInterpolationType::ApplyStandardPropertyValue(
ClampTime(property, To<InterpolableNumber>(interpolable_value).Value());
ComputedStyle& style = *state.Style();
switch (property) {
case CSSPropertyID::kHoverPopUpDelay:
style.SetHoverPopUpDelay(clamped_seconds);
case CSSPropertyID::kPopUpShowDelay:
style.SetPopUpShowDelay(clamped_seconds);
break;
case CSSPropertyID::kHoverPopUpHideDelay:
style.SetHoverPopUpHideDelay(clamped_seconds);
case CSSPropertyID::kPopUpHideDelay:
style.SetPopUpHideDelay(clamped_seconds);
break;
default:
NOTREACHED();
Expand Down
4 changes: 2 additions & 2 deletions blink/renderer/core/css/css_properties.json5
Original file line number Diff line number Diff line change
Expand Up @@ -2815,7 +2815,7 @@
valid_for_position_fallback: true,
},
{
name: "hover-pop-up-delay",
name: "pop-up-show-delay",
runtime_flag: "HTMLPopupAttribute",
property_methods: ["ParseSingleValue", "CSSValueFromComputedStyleInternal"],
field_group: "*",
Expand All @@ -2828,7 +2828,7 @@
supports_incremental_style: true,
},
{
name: "hover-pop-up-hide-delay",
name: "pop-up-hide-delay",
runtime_flag: "HTMLPopupAttribute",
property_methods: ["ParseSingleValue", "CSSValueFromComputedStyleInternal"],
field_group: "*",
Expand Down
8 changes: 4 additions & 4 deletions blink/renderer/core/css/css_property_equality.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,10 @@ bool CSSPropertyEquality::PropertiesEqual(const PropertyHandle& property,
return a.GridTemplateRows() == b.GridTemplateRows();
case CSSPropertyID::kHeight:
return a.Height() == b.Height();
case CSSPropertyID::kHoverPopUpDelay:
return a.HoverPopUpDelay() == b.HoverPopUpDelay();
case CSSPropertyID::kHoverPopUpHideDelay:
return a.HoverPopUpHideDelay() == b.HoverPopUpHideDelay();
case CSSPropertyID::kPopUpShowDelay:
return a.PopUpShowDelay() == b.PopUpShowDelay();
case CSSPropertyID::kPopUpHideDelay:
return a.PopUpHideDelay() == b.PopUpHideDelay();
case CSSPropertyID::kLeft:
return a.Left() == b.Left();
case CSSPropertyID::kLetterSpacing:
Expand Down
12 changes: 6 additions & 6 deletions blink/renderer/core/css/properties/longhands/longhands_custom.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3635,35 +3635,35 @@ const CSSValue* Height::CSSValueFromComputedStyleInternal(
style);
}

const CSSValue* HoverPopUpDelay::ParseSingleValue(
const CSSValue* PopUpShowDelay::ParseSingleValue(
CSSParserTokenRange& range,
const CSSParserContext& context,
const CSSParserLocalContext& local_context) const {
return css_parsing_utils::ConsumeTime(
range, context, CSSPrimitiveValue::ValueRange::kNonNegative);
}

const CSSValue* HoverPopUpDelay::CSSValueFromComputedStyleInternal(
const CSSValue* PopUpShowDelay::CSSValueFromComputedStyleInternal(
const ComputedStyle& style,
const LayoutObject* layout_object,
bool allow_visited_style) const {
return CSSNumericLiteralValue::Create(style.HoverPopUpDelay(),
return CSSNumericLiteralValue::Create(style.PopUpShowDelay(),
CSSPrimitiveValue::UnitType::kSeconds);
}

const CSSValue* HoverPopUpHideDelay::ParseSingleValue(
const CSSValue* PopUpHideDelay::ParseSingleValue(
CSSParserTokenRange& range,
const CSSParserContext& context,
const CSSParserLocalContext& local_context) const {
return css_parsing_utils::ConsumeTime(
range, context, CSSPrimitiveValue::ValueRange::kNonNegative);
}

const CSSValue* HoverPopUpHideDelay::CSSValueFromComputedStyleInternal(
const CSSValue* PopUpHideDelay::CSSValueFromComputedStyleInternal(
const ComputedStyle& style,
const LayoutObject* layout_object,
bool allow_visited_style) const {
return CSSNumericLiteralValue::Create(style.HoverPopUpHideDelay(),
return CSSNumericLiteralValue::Create(style.PopUpHideDelay(),
CSSPrimitiveValue::UnitType::kSeconds);
}

Expand Down
8 changes: 4 additions & 4 deletions blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3100,7 +3100,7 @@ Element* Element::PopupHoverTargetElement() const {
// within another pop-up, and the container pop-up is a descendant of this
// pop_up. For the special case of popup=manual pop-ups, which do not have
// ancestral relationships, this function checks pure DOM tree descendants of
// popup=manual pop-ups. This is important for the `hover-pop-up-hide-delay` CSS
// popup=manual pop-ups. This is important for the `pop-up-hide-delay` CSS
// property, which works for all pop-up types, and needs to keep pop-ups open
// when a descendant is hovered.
bool Element::IsNodePopUpDescendant(const Node& node) const {
Expand All @@ -3125,11 +3125,11 @@ bool Element::IsNodePopUpDescendant(const Node& node) const {
void Element::MaybeQueuePopupHideEvent() {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
DCHECK(HasValidPopupAttribute());
// If the pop-up isn't showing, or it has an infinite HoverPopUpHideDelay, do
// If the pop-up isn't showing, or it has an infinite PopUpHideDelay, do
// nothing.
if (GetPopupData()->visibilityState() == PopupVisibilityState::kHidden)
return;
float hide_delay_seconds = GetComputedStyle()->HoverPopUpHideDelay();
float hide_delay_seconds = GetComputedStyle()->PopUpHideDelay();
// If the value is infinite or NaN, don't hide the pop-up.
if (!std::isfinite(hide_delay_seconds))
return;
Expand Down Expand Up @@ -3194,7 +3194,7 @@ void Element::HandlePopupHovered(bool hovered) {
auto& hover_tasks = popup_element->GetPopupData()->hoverShowTasks();
DCHECK(!hover_tasks.Contains(this));

float hover_delay_seconds = GetComputedStyle()->HoverPopUpDelay();
float hover_delay_seconds = GetComputedStyle()->PopUpShowDelay();
// If the value is infinite or NaN, don't queue a task at all.
DCHECK_GE(hover_delay_seconds, 0);
if (std::isfinite(hover_delay_seconds)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,6 @@
]);
}

testprop('hover-pop-up-delay');
testprop('hover-pop-up-hide-delay');
testprop('pop-up-show-delay');
testprop('pop-up-hide-delay');
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
<style>
[popup] {
top:100px;
hover-pop-up-hide-delay: 100ms;
pop-up-hide-delay: 100ms;
}
[popuphovertarget] {
top:200px;
hover-pop-up-delay: 100ms;
pop-up-show-delay: 100ms;
}
#unrelated {top: 300px;}
div {
Expand Down Expand Up @@ -60,9 +60,9 @@
assert_false(popUp.matches(':top-layer'));
assert_true(msSinceMouseOver() >= hoverWaitTime,'waitForHoverTime should wait the specified time');
assert_true(hoverWaitTime > hoverDelays,'hoverDelays is the value from CSS, hoverWaitTime should be longer than that');
assert_equals(getComputedStyleTimeMs(invoker1,'hoverPopUpDelay'),hoverDelays,'hover-pop-up-delay is incorrect');
assert_equals(getComputedStyleTimeMs(popUp,'hoverPopUpHideDelay'),hoverDelays,'hover-pop-up-hide-delay is incorrect');
},`The hover-pop-up-hide-delay causes a pop-up to be hidden after a delay`);
assert_equals(getComputedStyleTimeMs(invoker1,'popUpShowDelay'),hoverDelays,'pop-up-show-delay is incorrect');
assert_equals(getComputedStyleTimeMs(popUp,'popUpHideDelay'),hoverDelays,'pop-up-hide-delay is incorrect');
},`The pop-up-hide-delay causes a pop-up to be hidden after a delay`);

promise_test(async (t) => {
await mouseOver(unrelated);
Expand Down Expand Up @@ -110,7 +110,7 @@
await mouseOver(unrelated);
const popUp = document.getElementById('example2');
const invoker = document.getElementById('invoker2');
assert_equals(getComputedStyleTimeMs(popUp,'hoverPopUpHideDelay'),hoverDelays,'hover-pop-up-hide-delay is incorrect');
assert_equals(getComputedStyleTimeMs(popUp,'popUpHideDelay'),hoverDelays,'pop-up-hide-delay is incorrect');
assert_false(popUp.matches(':top-layer'));
await mouseOver(invoker);
popUp.showPopUp();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@
</style>

<script>
const hoverPopUpDelay = 100; // The CSS delay setting.
const popUpShowDelay = 100; // The CSS delay setting.
const hoverWaitTime = 200; // How long to wait to cover the delay for sure.
let nextId = 0;
async function makePopUpAndInvoker(test, popUpType, invokerType, delayMs) {
delayMs = delayMs || hoverPopUpDelay;
delayMs = delayMs || popUpShowDelay;
const popUp = Object.assign(document.createElement('div'),{popUp: popUpType, id: `pop-up-${nextId++}`});
document.body.appendChild(popUp);
popUp.textContent = 'Pop-up';
let invoker = document.createElement('div');
invoker.setAttribute('class','invoker');
invoker.setAttribute('popuphovertarget',popUp.id);
invoker.setAttribute('style',`hover-pop-up-delay: ${delayMs}ms; hover-pop-up-hide-delay: 1000s;`);
invoker.setAttribute('style',`pop-up-show-delay: ${delayMs}ms; pop-up-hide-delay: 1000s;`);
document.body.appendChild(invoker);
const actualHoverDelay = Number(getComputedStyle(invoker)['hoverPopUpDelay'].slice(0,-1))*1000;
assert_equals(actualHoverDelay,delayMs,'hover-pop-up-delay is incorrect');
const actualHoverDelay = Number(getComputedStyle(invoker)['popUpShowDelay'].slice(0,-1))*1000;
assert_equals(actualHoverDelay,delayMs,'pop-up-show-delay is incorrect');
const originalInvoker = invoker;
const reassignPopupFn = (p) => originalInvoker.setAttribute('popuphovertarget',p.id);
switch (invokerType) {
Expand Down Expand Up @@ -95,12 +95,12 @@
await mouseOver(invoker);
let showing = popUp.matches(':top-layer');
// See NOTE above.
if (msSinceMouseOver() < hoverPopUpDelay)
if (msSinceMouseOver() < popUpShowDelay)
assert_false(showing,'pop-up should not show immediately');
await waitForHoverTime(hoverWaitTime);
assert_true(msSinceMouseOver() >= hoverWaitTime,'waitForHoverTime should wait the specified time');
assert_true(popUp.matches(':top-layer'),'pop-up should show after delay');
assert_true(hoverWaitTime > hoverPopUpDelay,'hoverPopUpDelay is the CSS setting, hoverWaitTime should be longer than that');
assert_true(hoverWaitTime > popUpShowDelay,'popUpShowDelay is the CSS setting, hoverWaitTime should be longer than that');
popUp.hidePopUp(); // Cleanup
},`popuphovertarget attribute shows a pop-up with popup=${type}, invokerType=${invokerType}`);

Expand All @@ -118,7 +118,7 @@
if (msSinceMouseOver() >= longerHoverDelay)
return; // The WPT runner was too slow.
assert_false(showing,'pop-up should not show after not long enough of a delay');
},`popuphovertarget hover-pop-up-delay is respected (popup=${type}, invokerType=${invokerType})`);
},`hoverpopup pop-up-show-delay is respected (popup=${type}, invokerType=${invokerType})`);

promise_test(async (t) => {
const {popUp,invoker} = await makePopUpAndInvoker(t,type,invokerType);
Expand All @@ -137,7 +137,7 @@
let showing = popUp.matches(':top-layer');
popUp.remove();
// See NOTE above.
if (msSinceMouseOver() >= hoverPopUpDelay)
if (msSinceMouseOver() >= popUpShowDelay)
return; // The WPT runner was too slow.
assert_false(showing,'pop-up should not show immediately');
await waitForHoverTime(hoverWaitTime);
Expand All @@ -157,7 +157,7 @@
let eitherShowing = popUp.matches(':top-layer') || popUp2.matches(':top-layer');
reassignPopupFn(popUp2);
// See NOTE above.
if (msSinceMouseOver() >= hoverPopUpDelay)
if (msSinceMouseOver() >= popUpShowDelay)
return; // The WPT runner was too slow.
assert_false(eitherShowing,'pop-up should not show immediately');
await waitForHoverTime(hoverWaitTime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,6 @@ grid-template-areas: none
grid-template-columns: none
grid-template-rows: none
height: 0px
hover-pop-up-delay: 0.5s
hover-pop-up-hide-delay: infinity * 1s
hyphens: manual
image-orientation: from-image
image-rendering: auto
Expand Down Expand Up @@ -281,6 +279,8 @@ paint-order: normal
perspective: none
perspective-origin: 384.5px 0px
pointer-events: auto
pop-up-hide-delay: infinity * 1s
pop-up-show-delay: 0.5s
position: static
position-fallback: none
r: 0px
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,6 @@ grid-template-areas: none
grid-template-columns: none
grid-template-rows: none
height: auto
hover-pop-up-delay: 0.5s
hover-pop-up-hide-delay: infinity * 1s
hyphens: manual
image-orientation: from-image
image-rendering: auto
Expand Down Expand Up @@ -281,6 +279,8 @@ paint-order: normal
perspective: none
perspective-origin: 50% 50%
pointer-events: auto
pop-up-hide-delay: infinity * 1s
pop-up-show-delay: 0.5s
position: static
position-fallback: none
r: 0px
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,6 @@ grid-template-areas: none
grid-template-columns: none
grid-template-rows: none
height: 100px
hover-pop-up-delay: 0.5s
hover-pop-up-hide-delay: infinity * 1s
hyphens: manual
image-orientation: from-image
image-rendering: auto
Expand Down Expand Up @@ -281,6 +279,8 @@ paint-order: normal
perspective: none
perspective-origin: 0px 0px
pointer-events: auto
pop-up-hide-delay: infinity * 1s
pop-up-show-delay: 0.5s
position: static
position-fallback: none
r: 0px
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,6 @@ gridTemplateAreas
gridTemplateColumns
gridTemplateRows
height
hoverPopUpDelay
hoverPopUpHideDelay
hyphens
imageOrientation
imageRendering
Expand Down Expand Up @@ -339,6 +337,8 @@ placeContent
placeItems
placeSelf
pointerEvents
popUpHideDelay
popUpShowDelay
position
positionFallback
prefix
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,6 @@ All changes to this list should go through Blink's feature review process: http:
grid-template-columns
grid-template-rows
height
hover-pop-up-delay
hover-pop-up-hide-delay
hyphens
image-orientation
image-rendering
Expand Down Expand Up @@ -301,6 +299,8 @@ All changes to this list should go through Blink's feature review process: http:
perspective
perspective-origin
pointer-events
pop-up-hide-delay
pop-up-show-delay
position
position-fallback
quotes
Expand Down

0 comments on commit 2b4b30f

Please sign in to comment.