feat(Page): add high contrast border by kmcfaul · Pull Request #7718 · patternfly/patternfly

kmcfaul · 2025-08-01T19:27:04Z

Closes #7590.

@mcoker / @srambach Would this approach of updating the existing border on the page main container work for this use case or would the before/after approach still be preferred? I had initially run into some z-indexing issues on the border & page content as well as seeing the two borders concurrently, before I changed track and updated the current border instead because it already existed. But I can definitely keep adjusting the after block if that's still preferred, I think I just need to disable the current border and then resolve the z-indexes.

With the current approach, there is also a 2px difference in the border widths (4px on the normal border and 2px on the high contrast border) that's causing a small layout shift and I'm unsure the best way to solve it. Maybe adding additional padding when high contrast is on?

patternfly-build · 2025-08-01T19:30:54Z

Preview: https://pf-pr-7718.surge.sh

A11y report: https://pf-pr-7718-a11y.surge.sh

kmcfaul · 2025-08-04T16:32:20Z

Updated to prevent the layout shift -

Other page elements will account for the difference in border width when calcing their padding.
There is now a small amount (2px) of page-main-container padding only in high contrast mode to compensate for the border width difference in layout.

lboehling

🌟

srambach · 2025-08-18T14:49:50Z

  --#{$page}__main-container--BorderRadius: var(--pf-t--global--border--radius--medium);
  --#{$page}__main-container--BorderWidth: #{pf-size-prem(4px)}; // TODO Change to be a page outline token
  --#{$page}__main-container--BorderColor: var(--#{$page}__main-container--BackgroundColor); // TODO Border should match the background to blend in - change to be a page outline token
+  --#{$page}__main-container--BorderOffset: var(--#{$page}__main-container--BorderWidth);


Since there isn't a css attribute called border-offset, wdyt about calling this --#{$page}__main-container--Padding--offset? It looks like we've used that pattern once or twice before. @mcoker ?

srambach · 2025-08-20T19:25:29Z

+    --#{$page}__main-container--BorderWidth: var(--pf-t--global--border--width--strong);
+    --#{$page}__main-container--BorderColor: var(--pf-t--global--border--color--high-contrast);
+    --#{$page}__main-container--Padding--offset: #{pf-size-prem(4px)};


@lboehling @mcoker We have a comment in the code saying that we should create page border outline width and color tokens. Is this a good time to do that? Then it could adjust in the high-contrast tokens. Trying to see if we could get rid of this media query! Since the offset is 4px either way, maybe that would do it?

I like that! @lboehling do you have any issues with adding that to figma?

That would also let us use that token value to offset any component padding/insets that might need to take into consideration that border width to align properly with other page chrome elements. I haven't looked too much into it, but I'm wondering if that might help resolve this issue with tabs not aligning properly with the page chrome gutter #7376

mcoker

@kmcfaul design added tokens for the page border color (--pf-t--global--border--color--page--default) and border width (--pf-t--global--border--width--page--default) in #7761.

Would you mind rebasing and dropping that token in here where appropriate? I'm hoping it can get rid of the custom @media (prefers-contrast) styles like @srambach mentioned in https://github.com/patternfly/patternfly/pull/7718/files#r2289094152

kmcfaul · 2025-08-26T16:15:11Z

@mcoker Rebased and updated to use the new tokens, but the media-query may still be required to account for the border width/padding issue. And speaking of that, I'm seeing a small layout shift again even though the calculated sizes should be the same as the previous implementation. Do you have any ideas for addressing this shift?

mcoker · 2025-08-26T19:34:39Z

And speaking of that, I'm seeing a small layout shift again even though the calculated sizes should be the same as the previous implementation. Do you have any ideas for addressing this shift?

@kmcfaul are you referring to a shift in either the subnav or breadcrumb container? That's where I'm seeing a shift. Assuming that's the case, I believe it's because of a scoping issue with the change of --#{$page}__main-container--Padding--offset in this block

patternfly/src/patternfly/components/Page/page.scss

Lines 388 to 392 in 1cdf818

    
           @media (prefers-contrast: more) { 
        
             --#{$page}__main-container--Padding--offset: #{pf-size-prem(4px)}; 
        
             padding: calc(var(--#{$page}__main-container--Padding--offset) - var(--#{$page}__main-container--BorderWidth)); 
        
           }

If you update the value of a CSS var, you'll need to update it in the same scope of where it's declared/used. Meaning you'd want to move the @media query up to the same scope as where all of the component variables that use --#{$page}__main-container--Padding--offset are defined. What I'm not sure on is whether a change within a @media query in a scope will also apply to other vars in that same scope but outside of the @media query, so maybe give it a shot and let me know if it doesn't work?

The reason the main-section padding doesn't shift is because --#{$page}__main-container--Padding--offset is used within the same scope as the change you made on the main container element.

patternfly/src/patternfly/components/Page/page.scss

Lines 502 to 503 in 1cdf818

    
           padding-inline-start: calc(var(--#{$page}__main-section--PaddingInlineStart) - var(--#{$page}__main-container--Padding--offset)); 
        
           padding-inline-end: calc(var(--#{$page}__main-section--PaddingInlineEnd) - var(--#{$page}__main-container--Padding--offset));

kmcfaul · 2025-08-28T14:58:04Z

Update - the layout shift must be something with the examples page / seeing the various inline page examples. In a full page example in a new tab, there is no layout shift. So this PR should be good to go unless anyone finds any other gotchas.

srambach · 2025-08-29T13:30:01Z

+  --#{$page}__main-container--BorderWidth: var(--pf-t--global--border--width--page--default);
+  --#{$page}__main-container--BorderColor: var(--pf-t--global--border--color--page--default); 
+  --#{$page}__main-container--Padding--offset: 4px;


Suggested change

--#{$page}__main-container--BorderWidth: var(--pf-t--global--border--width--page--default);

--#{$page}__main-container--BorderColor: var(--pf-t--global--border--color--page--default);

--#{$page}__main-container--Padding--offset: 4px;

--#{$page}__main-container--BorderWidth: var(--pf-t--global--border--width--page--default);

--#{$page}__main-container--BorderColor: var(--pf-t--global--border--color--page--default);

I think that we can just make this change and revert all the other changes to the file. I don't think we need the offset because subtracting the border width from the page section(s) padding (which was already done) will give the correct spacing and avoid the 2px gap between the high-contrast border and a section with a secondary background. There will be a slight vertical shift as the top border goes from 4 to 2px, but I think that's acceptable as we currently don't try to vertically align the main area with anything else.

If we want to avoid the vertical shift, we can select the first page group/section and adjust, but it's a bit messy and perhaps not worth it.

@lboehling @mcoker

i'm good with this 👍 thank you!

follow up Q -- we had discussed updating --pf-t--global--border--width--page--default & --pf-t--global--border--color--page--default to use "main" instead of "page" do we still want to do that? @mcoker @srambach

I don't hate the idea of that 😄 But I don't consider it a blocker to this.

I like it. We could then use page areas as names - "header", "sidebar", possibly "nav" if needed, and "main". Not a blocker to this PR, which I'd like to get merged ASAP but we'd want to make that change this sprint before code freeze.

sounds good -- i'll make that change in figma so it's ready the next time we export the tokens.

lboehling

looks good!

mcoker

LGTM!

srambach

L 🍾 TM!

kmcfaul linked an issue Aug 4, 2025 that may be closed by this pull request

High contrast borders - Page #7590

Closed

mcoker requested review from lboehling, mcoker and srambach August 7, 2025 23:24

lboehling approved these changes Aug 15, 2025

View reviewed changes

srambach reviewed Aug 18, 2025

View reviewed changes

kmcfaul force-pushed the page-high-contrast branch from 714bff0 to 7901993 Compare August 20, 2025 15:00

srambach reviewed Aug 20, 2025

View reviewed changes

mcoker mentioned this pull request Aug 25, 2025

Pull in latest from design tokens repo #7760

Closed

mcoker requested changes Aug 25, 2025

View reviewed changes

kmcfaul added 4 commits August 26, 2025 10:47

feat(Page): add high contrast border

fc01913

fix(): update layout to prevent shift

c716e1b

feat(Page): update --BorderOffset to --Padding--offset

d361a5d

feat(Page): use new tokens

1cdf818

kmcfaul force-pushed the page-high-contrast branch from 7901993 to 1cdf818 Compare August 26, 2025 16:13

feat(Page): remove media query in favor of calc

350f24c

srambach reviewed Aug 29, 2025

View reviewed changes

feat(Page): remove Padding--offset

31be2fd

mcoker requested a review from lboehling September 2, 2025 15:56

mcoker mentioned this pull request Sep 2, 2025

High contrast - pull latest tokens #7779

Closed

lboehling approved these changes Sep 2, 2025

View reviewed changes

mcoker approved these changes Sep 2, 2025

View reviewed changes

srambach approved these changes Sep 2, 2025

View reviewed changes

mcoker merged commit 9793557 into patternfly:high-contrast-q3 Sep 2, 2025
4 checks passed

mcoker mentioned this pull request Sep 4, 2025

High contrast borders - Page #7590

Closed

Conversation

kmcfaul commented Aug 1, 2025

Uh oh!

patternfly-build commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kmcfaul commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lboehling left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

kmcfaul commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcoker commented Aug 26, 2025

Uh oh!

kmcfaul commented Aug 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lboehling Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lboehling left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

srambach left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

patternfly-build commented Aug 1, 2025 •

edited

Loading

kmcfaul commented Aug 4, 2025 •

edited

Loading

kmcfaul commented Aug 26, 2025 •

edited

Loading

lboehling Aug 29, 2025 •

edited

Loading