Skip to content

Commit

Permalink
pf: Make Firefox form padding consistent with other browsers
Browse files Browse the repository at this point in the history
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
  • Loading branch information
garrett authored and martinpitt committed Mar 12, 2025
1 parent c86c551 commit 01a51a9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
28 changes: 15 additions & 13 deletions pkg/lib/patternfly/patternfly-5-overrides.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 01a51a9

Please sign in to comment.