Skip to content

fix(TOC): fixes toc showing incorrectly at exactly 1450px#4802

Merged
rebeccaalpert merged 6 commits into
patternfly:mainfrom
srambach:4747-toc-responsiveness
Oct 13, 2025
Merged

fix(TOC): fixes toc showing incorrectly at exactly 1450px#4802
rebeccaalpert merged 6 commits into
patternfly:mainfrom
srambach:4747-toc-responsiveness

Conversation

@srambach
Copy link
Copy Markdown
Member

Fixes #4747

This fixes the small blip where the menu shows incorrectly at exactly 1450px.

One known problem: if the TOC is expanded when in the dropdown (mobile) mode, then the .pf-m-expanded class will be added. If the window is resized to be larger than 1450px, the TOC will be displayed vertically - however, that class is never removed. So the next time the user resizes the window to be smaller than 1450px, the expanded class is still there and the menu will show as expanded.

@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented Sep 25, 2025

Preview: https://pf-org--pr-4802-site.surge.sh

Copy link
Copy Markdown
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table of contents spacing looks different to me here, but I am on a smaller screen, so take this with a grain of salt:

1451 1450
Screenshot 2025-09-26 at 2 41 08 PM Screenshot 2025-09-26 at 2 41 01 PM

I think this was what Parthiv's margin stuff was supposed to fix.

I'm wondering if we don't need to address https://github.com/patternfly/patternfly-org/blob/main/packages/documentation-framework/components/tableOfContents/tableOfContents.js#L99 and/or https://github.com/patternfly/patternfly-org/pull/4802/files#diff-2bc5461d65edc211f100c8082b2a58df1044e3f682759af7a27228f7c94288c1L75 as well if we are doing this. Probably the min-width needs to be adjusted and we probably need a >= in the .js. I haven't tried this, but I suspect that's relevant as well.

@srambach
Copy link
Copy Markdown
Member Author

@rebeccaalpert You were right about the other media query; that fixed the difference in spacing. All the numbers were off by 1px and that's based on our 2xl breakpoint because the jumplinks have the .pf-m-non-expandable-on-2xl class; 2xl is 90.625rem, or 1450px.

I also adjusted the offset to add a condition because our page also reconfigures slightly at the md breakpoint of 768px, so we needed a different offset value between 768 and 1450.

Finally, @lboehling I tightened up the spacing around the TOC toggle from a -md block padding to a -sm because it seemed so dang big ;-) Happy to undo that if you don't like it!

One last comment..the background color on the mobile version makes it appear that you can click anywhere, but you can really only click on the button, which is only as long as the text. I toyed with making the button full width or turning off the background but left it as is, so if you think it would be good to change that and have ideas, lmk!

Copy link
Copy Markdown
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good; thank you!

My only comment is a style thing - technically it works. On this line, it's probably better to break up the nested ternary operator: https://github.com/patternfly/patternfly-org/pull/4802/files#diff-150f1ee54d93b6a1323d99e56a2963cf63ab097113bf89e9d6eac67a90d787f1R99

It's just harder to read. Typically I'll see nested one-liners like this reworked into a switch statement or a function (getOffset or similar).

@srambach srambach self-assigned this Sep 29, 2025
Copy link
Copy Markdown
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

She's a beaut!

Copy link
Copy Markdown
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned offline but looks like there is still a ~11px or so viewport width where the TOC drops below the main content. I'm thinking it might have to do with the TOC width

and the content area max-width

.ws-example-page-wrapper {
max-width: min(100%, 825px);
}

and the flex wrapper for them, which will wrap by default

<div className={source !== 'landing-pages' ? 'pf-v6-l-flex' : ''}>
{toc.length > 1 && <TableOfContents items={toc} />}
<Stack hasGutter className={(source !== 'landing-pages' && 'ws-example-page-wrapper')}>

I'm wondering if maybe we should add .pf-m-nowrap to that flex layout to keep them from wrapping when they're in a row, and .pf-m-row-on-2xl to switch the layout to a row at the same point the TOC/jump links switches from mobile to desktop?

clip.mov

Copy link
Copy Markdown
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! 💃

Copy link
Copy Markdown
Contributor

@bekah-stephens bekah-stephens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨👌🏻

Copy link
Copy Markdown
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@rebeccaalpert rebeccaalpert merged commit de0c2b4 into patternfly:main Oct 13, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug - PF.org table of contents responsiveness

6 participants