Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sticky breadcrumb and sidebar navs #1198

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

davidmcguiredesign
Copy link
Contributor

@davidmcguiredesign davidmcguiredesign commented Jan 5, 2025

Both the breadcrumb (parent) and sidebar (sibling) navs provide key context while reading the page. Making them sticky (and optionally giving the sidebar nav its own scroll context) makes their context available to the whole page, not just the start.

close #1197

Both the breadcrumb (parent) and sidebar (sibling) navs provide key context while reading the page. Making them sticky (and optionally giving the sidebar nav its own scroll context) makes their context available to the whole page, not just the start.
Copy link
Contributor

github-actions bot commented Jan 5, 2025

🚀 Preview for commit 2cf9118 can be found at https://web-php-pr-1198.preview.thephp.foundation

Copy link
Contributor

github-actions bot commented Jan 5, 2025

🚀 Regression report for commit 2cf9118 is at https://web-php-regression-report-pr-1198.preview.thephp.foundation

@davidmcguiredesign
Copy link
Contributor Author

All the failed tests (with the possible exception of the archives page) appear to be because the rounded corner does not match the screenshots. This is expected behavior.

The sticky breadcrumb nav makes it slightly harder to tell when the page top has been reached, so I added a single round corner to signal the content's upper limit.

@saundefined
Copy link
Member

Hi, thanks for the PR!

I've updated the snapshots so they don't fall off due to the wrong year,
the only ones left are the ones related to rounding corner.

Is this rounding due to UI practices, or is there some technical issue?

If the former, I'm not good at it, so I can't judge (but to me a right angle or a rounded one fits equally),
I'd like to hear others' opinions.

If the second, we can update the snapshots for tests, including rounding.

@davidmcguiredesign
Copy link
Contributor Author

The rounded corner is to preemptively solve a minor UX concern introduced by the sticky layout, not to solve a technical issue. I'd be happy to remove that line from my branch if it is a sticking point (no pun intended) or if no one else sees the concern.

@saundefined
Copy link
Member

All right! Let's wait for feedback on the rounding
(or we can put it in a separate PR, and not wait, I guess)

BTW, do you think maybe adding sticky to similar pages as well?
https://web-php-pr-1198.preview.thephp.foundation/downloads.php

@davidmcguiredesign
Copy link
Contributor Author

The way I see it, the other pages are split between a maybe-so and a probably-not.

  • The downloads page might benefit from it.
  • The index page might benefit from it, but only because there’s so little in the sidebar. Stack Overflow makes such great use of their sidebar, that I wonder if PHP’s is being under-utilized.
  • There’s so much in the sidebar of the conferences page that separating that sidebar from the main scroll would probably be worse.
Could the conferences page be solved?

Rendering the conferences’ sidenav as a tree of years (see example below) would probably be a good thing, but if a tree pattern can be applied here, why not in the docs, which actually are a tree? But if the tree pattern is applied to the docs, how deep does the tree go? Would all pages go in? Would it follow a more complex pattern like MDN Web Docs, with a combination of siblings, descendants, and cousins? Probably a fruitful line of inquiry, but beyond this scope.

<details open class='year'>
    <summary>2025</summary>
    <a>International PHP Conference Berlin 2025</a>
    <a>ConFoo 2025</a>
    <a>phpday 2025 - Call For Papers</a>
    <!-- ...etc... -->
</details>
<details class='year'>
    <summary>2024</summary>
    <a>API Platform Conference 2024</a>
    <a>PHP Russia 2024</a>
    <a>ConFoo Montreal 2025: Call for Papers</a>
    <a>CascadiaPHP 2024</a>
    <!-- ...etc... -->
</details>
<!-- ...etc... -->

<style>
    .year > a { display: block; }
</style>

@saundefined
Copy link
Member

Indeed, let's postpone this issue for the future :)

So, LGTM, just need to either remove rounding, or update snapshots.
@sy-records what do you think?

@davidmcguiredesign
Copy link
Contributor Author

I'll take that as a "no" (or at least a "not yet") on the border radius. I rolled back that change and pulled in the latest version of the master branch.

Copy link
Member

@saundefined saundefined left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@saundefined saundefined merged commit f843f56 into php:master Jan 27, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sticky in-manual navs
3 participants