From 56adb6680af79ae75e06ecf0399dfe8b2dcceed0 Mon Sep 17 00:00:00 2001 From: Garrett LeSage Date: Thu, 6 Mar 2025 10:26:05 +0100 Subject: [PATCH] pf: Make Firefox form padding consistent with other browsers It seems Firefox fixed their longstanding bug, which caused the workaround in PatternFly (who adopted it from us) to no longer be relevant. I adjusted our own workaround, but it didn't apply as PatternFly also (at some point in time) changed the DOM so the selector no longer matched. We'll want to communicate this with PF to get PF6 fixed. Possibly even PF5 too, as it apparently changed in Firefox recently, and therefore newer versions of Firefox will "break" the expected layout without addressing it in PF directly (or with a workaround in codebases). Meanwhile, overriding the override by setting the left spacing to be the standard left spacing makes sense too. And we can just set this for Firefox-only instead of everything (which would still work, of course, but let's scope this to the scope of the issue related to the workaround). Fixes #21671 https://issues.redhat.com/browse/RHEL-12150 --- .../patternfly/patternfly-5-overrides.scss | 28 ++++++++++--------- test/reference | 2 +- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/lib/patternfly/patternfly-5-overrides.scss b/pkg/lib/patternfly/patternfly-5-overrides.scss index 9b6e7154f4d2..06469d24d7b8 100644 --- a/pkg/lib/patternfly/patternfly-5-overrides.scss +++ b/pkg/lib/patternfly/patternfly-5-overrides.scss @@ -53,19 +53,21 @@ to wrap around when there isn't enough space. overflow-y: auto; } -/* Adjust padding on form selects to resemble PF non-form selects */ -/* (This can be seen when the longest text is selected on a non-stretched select) */ -/* Upstream: https://github.com/patternfly/patternfly/issues/4387 */ -/* Cockpit-Podman: https://github.com/cockpit-project/cockpit-podman/issues/755 */ -select.pf-v5-c-form-control { - --pf-v5-c-form-control--PaddingRight: 41px; - --pf-v5-c-form-control--PaddingLeft: 8px; - - // Firefox's select text has additional padding (4px) - /* stylelint-disable-next-line at-rule-no-vendor-prefix */ - @-moz-document url-prefix() { - --pf-v5-c-form-control--PaddingRight: 37px; - --pf-v5-c-form-control--PaddingLeft: 4px; +/* New issue based on the regression based on workaround causing this issue: https://github.com/cockpit-project/cockpit/issues/21671 +*/ +/* Old upstream: https://github.com/patternfly/patternfly/issues/4387 */ +/* Old Cockpit-Podman: https://github.com/cockpit-project/cockpit-podman/issues/755 */ +.pf-v5-c-form-control > select { + /* Right side needs to compensate for the icon and space around it, + for all browsers */ + --pf-v5-c-form-control--PaddingRight: calc( + var(--pf-v5-global--icon--FontSize--md) + var(--pf-v5-global--spacer--lg) + ); + + @-moz-document url-prefix() { // stylelint-disable-line at-rule-no-vendor-prefix + /* "Unset" Firefox special handling in PatternFly; using consistent + spacing with others; this can be removed after PatternFly removes it */ + --pf-v5-c-form-control--PaddingLeft: var(--pf-v5-c-form-control--inset--base); } } diff --git a/test/reference b/test/reference index 17d582c0d186..f2ff51fea8bf 160000 --- a/test/reference +++ b/test/reference @@ -1 +1 @@ -Subproject commit 17d582c0d186891dc71ef21933305112fae8ea5a +Subproject commit f2ff51fea8bf5abd7ec0b37b27e36339cb0c1063