Skip to content

fix(nav): update html examples to display indicator correctly. - #8557

Merged
mcoker merged 3 commits into
mainfrom
bug/8489-docked-nav-example-container
Aug 19, 2026
Merged

fix(nav): update html examples to display indicator correctly.#8557
mcoker merged 3 commits into
mainfrom
bug/8489-docked-nav-example-container

Conversation

@jcmill

@jcmill jcmill commented Aug 14, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Fixes: #8489

Adds a margin to docked nav and other nav HTML examples so the current-page indicator isn’t clipped. Applies margin-inline-start on the preview container, and skips expanded, horizontal, and drilldown variants that don’t need it.

Summary by CodeRabbit

  • Style
    • Added consistent medium inline spacing to navigation previews.
    • Updated horizontal navigation previews with the secondary default background.
    • Improved alignment and visual consistency across navigation examples and previews.

@coderabbitai

coderabbitai Bot commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: da206dac-83a8-46e7-9f93-7243f94bd0d7

📥 Commits

Reviewing files that changed from the base of the PR and between f90e52d and eec80da.

📒 Files selected for processing (1)
  • src/patternfly/components/Nav/examples/Navigation.css

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

The navigation example CSS adds medium inline padding to preview containers, changes the horizontal preview background token, and removes conditional inline-start margin.

Changes

Navigation preview styling

Layer / File(s) Summary
Preview styling rules
src/patternfly/components/Nav/examples/Navigation.css		 Navigation preview containers receive medium inline-start and inline-end padding. The horizontal preview uses the secondary default background token. The conditional inline-start margin is removed.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to eec80

This localized styling change adjusts navigation example spacing so the current-page indicator displays correctly; no actionable merge-blocking risk remains.

Possibly related PRs

  • patternfly/patternfly#8330: Both changes update navigation styling, including navigation example presentation and docked-state selectors.

Suggested labels: released on @prerelease``

Suggested reviewers: andrew-ronaldson, bekah-stephens

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The changes modify navigation preview CSS but do not add the required container to the docked navigation HTML example [#8489]. Add the required container to the docked navigation HTML example and verify that the current-page indicator is not clipped.
Out of Scope Changes check ⚠️ Warning The horizontal navigation background token change is unrelated to the linked docked navigation container objective [#8489]. Remove the unrelated horizontal navigation background change or link it to a separate issue.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows conventional commit syntax and describes the navigation indicator fix.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/patternfly/components/Nav/examples/Navigation.css

Parsing error: Private names are only allowed in property accesses (obj.#ws) or in in expressions (#ws in obj). (1:0)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@patternfly-build

patternfly-build commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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

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

mcoker
mcoker reviewed Aug 18, 2026
View reviewed changes

@mcoker mcoker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a comment, tho I'll defer to whatever design thinks is best.

Comment thread src/patternfly/components/Nav/examples/Navigation.css Outdated
mcoker
mcoker reviewed Aug 19, 2026
View reviewed changes

@mcoker mcoker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This LGTM. What do you think about adding the background to horizontal (but not horizontal subnav)?

You can't see the hover/current background changes currently. Here's what it looks like now

Image

Here's with the background added

Image

@mcoker
mcoker merged commit 9f75a02 into main Aug 19, 2026
6 checks passed
@patternfly-build

patternfly-build commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 6.6.0-prerelease.36 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jcmill
jcmill deleted the bug/8489-docked-nav-example-container branch August 24, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcoker mcoker mcoker approved these changes
@bekah-stephens bekah-stephens bekah-stephens approved these changes
@andrew-ronaldson andrew-ronaldson Awaiting requested review from andrew-ronaldson

Assignees

No one assigned

Labels

released on @prerelease

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Docked nav: HTML example has no container

4 participants

@jcmill @patternfly-build @mcoker @bekah-stephens