Commit f5181777 authored by Mason Freed's avatar Mason Freed Committed by Commit Bot

Take min-width into account for buttons on Mac

Prior to this CL, buttons with min-width set did not honor the
minimum width, and instead were sized to fit their content. This
should now be fixed, and there is now a test to verify.

Bug: 428479
Change-Id: I717469db1a1affead5bebf5f1ad3b8f01cce0d92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1597279Reviewed-by: default avatarAleks Totic <atotic@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Auto-Submit: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658406}
parent 0196be28
...@@ -272,7 +272,8 @@ class CORE_EXPORT LayoutTheme : public RefCounted<LayoutTheme> { ...@@ -272,7 +272,8 @@ class CORE_EXPORT LayoutTheme : public RefCounted<LayoutTheme> {
// Returns the minimum size for a control in zoomed coordinates. // Returns the minimum size for a control in zoomed coordinates.
virtual LengthSize MinimumControlSize(ControlPart, virtual LengthSize MinimumControlSize(ControlPart,
const FontDescription&, const FontDescription&,
float /*zoomFactor*/) const { float /*zoomFactor*/,
const ComputedStyle& style) const {
return LengthSize(Length::Fixed(0), Length::Fixed(0)); return LengthSize(Length::Fixed(0), Length::Fixed(0));
} }
......
...@@ -183,7 +183,8 @@ class LayoutThemeMac final : public LayoutTheme { ...@@ -183,7 +183,8 @@ class LayoutThemeMac final : public LayoutTheme {
float zoom_factor) const override; float zoom_factor) const override;
LengthSize MinimumControlSize(ControlPart, LengthSize MinimumControlSize(ControlPart,
const FontDescription&, const FontDescription&,
float zoom_factor) const override; float zoom_factor,
const ComputedStyle& style) const override;
LengthBox ControlPadding(ControlPart, LengthBox ControlPadding(ControlPart,
const FontDescription&, const FontDescription&,
......
...@@ -1231,11 +1231,12 @@ LengthSize LayoutThemeMac::GetControlSize( ...@@ -1231,11 +1231,12 @@ LengthSize LayoutThemeMac::GetControlSize(
LengthSize LayoutThemeMac::MinimumControlSize( LengthSize LayoutThemeMac::MinimumControlSize(
ControlPart part, ControlPart part,
const FontDescription& font_description, const FontDescription& font_description,
float zoom_factor) const { float zoom_factor,
const ComputedStyle& style) const {
switch (part) { switch (part) {
case kSquareButtonPart: case kSquareButtonPart:
case kButtonPart: case kButtonPart:
return LengthSize(Length::Fixed(0), return LengthSize(style.MinWidth().Zoom(zoom_factor),
Length::Fixed(static_cast<int>(15 * zoom_factor))); Length::Fixed(static_cast<int>(15 * zoom_factor)));
case kInnerSpinButtonPart: { case kInnerSpinButtonPart: {
IntSize base = StepperSizes()[NSMiniControlSize]; IntSize base = StepperSizes()[NSMiniControlSize];
...@@ -1245,7 +1246,7 @@ LengthSize LayoutThemeMac::MinimumControlSize( ...@@ -1245,7 +1246,7 @@ LengthSize LayoutThemeMac::MinimumControlSize(
} }
default: default:
return LayoutTheme::MinimumControlSize(part, font_description, return LayoutTheme::MinimumControlSize(part, font_description,
zoom_factor); zoom_factor, style);
} }
} }
...@@ -1414,14 +1415,13 @@ void LayoutThemeMac::AdjustControlPartStyle(ComputedStyle& style) { ...@@ -1414,14 +1415,13 @@ void LayoutThemeMac::AdjustControlPartStyle(ComputedStyle& style) {
// Width / Height // Width / Height
// The width and height here are affected by the zoom. // The width and height here are affected by the zoom.
// FIXME: Check is flawed, since it doesn't take min-width/max-width
// into account.
LengthSize control_size = GetControlSize( LengthSize control_size = GetControlSize(
part, style.GetFont().GetFontDescription(), part, style.GetFont().GetFontDescription(),
LengthSize(style.Width(), style.Height()), style.EffectiveZoom()); LengthSize(style.Width(), style.Height()), style.EffectiveZoom());
LengthSize min_control_size = MinimumControlSize( LengthSize min_control_size =
part, style.GetFont().GetFontDescription(), style.EffectiveZoom()); MinimumControlSize(part, style.GetFont().GetFontDescription(),
style.EffectiveZoom(), style);
// Only potentially set min-size to |control_size| for these parts. // Only potentially set min-size to |control_size| for these parts.
if (part == kCheckboxPart || part == kRadioPart) if (part == kCheckboxPart || part == kRadioPart)
......
<!DOCTYPE html>
<html>
<meta charset="utf-8">
<title>min-width: Should apply to buttons</title>
<link rel="author" title="Mason Freed" href="mailto:masonfreed@chromium.org">
<link rel="help" href="https://www.w3.org/TR/CSS2/visudet.html#min-max-widths">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
.button-row {
background-color: #BBB;
display: flex;
flex-direction: row;
width: 400px;
margin-top: 8px;
}
button {
flex: 0 0 auto;
margin: 0px;
}
</style>
<div>
<p>Expected: All buttons should be 200px wide</p>
</div>
<div class="button-row">
<button style="min-width: 200px">Foo</button>
<button style="min-width: 200px">Bar</button>
</div>
<div class="button-row">
<button style="min-width: 50%">Foo</button>
<button style="min-width: 50%">Bar</button>
</div>
<div class="button-row" style="writing-mode: vertical-rl; height: 100px;flex-direction:column">
<button style="min-width: 200px">Foo</button>
<button style="min-width: 200px">Bar</button>
</div>
<div class="button-row" style="zoom:2">
<button style="min-width: 200px">Foo</button>
<button style="min-width: 200px">Bar</button>
</div>
<script>
test(() => {
let buttons = document.querySelectorAll("button");
for (let button of document.querySelectorAll("button"))
assert_equals(button.offsetWidth, 200);
}, 'min-width should force all buttons to be 200px wide');
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment