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

Allow top navbar to be scrolled away on most pages #1968

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Conversation

zarvox
Copy link
Contributor

@zarvox zarvox commented Jan 11, 2024

The always-visible topbar is actually not the best use of space on most pages:

  • the topbar is easily found by scrolling back to the top of the page
  • not having it take up 50 pixels of vertical space means 50 more rows for more-useful content like the puzzle list or guess queue
  • having the top bar scroll away means we don't have to worry as much about having it be strongly-contrasting in color with other content that would slide under the fixed navbar.
  • while some SPAs do have a more fixed layout, most other websites that have a navbar like this either do not make it sticky at all, or at least hide it when scrolling down

So let's just make the topbar be scrollable like most of the rest of the web, and give a little more space for content.

The view on PuzzlePage will not change at all, since we use a more fixed layout there anyway to position/size the chat and iframe.

@zarvox zarvox requested a review from ebroder January 11, 2024 06:16
@zarvox zarvox enabled auto-merge January 11, 2024 06:18
@@ -319,12 +324,15 @@ const App = ({ children }: { children: React.ReactNode }) => {
</ReactErrorBoundary>
);
}

const onPuzzlePage = useParams<"puzzleId">().puzzleId !== undefined;
Copy link
Member

Choose a reason for hiding this comment

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

I don't really love this? It feels fragile and only tangentially connected to the question we're actually trying to answer. (What if we add another page with a puzzleId?)

On the other hand, I'm not sure we need it? I see two issues:

First, the border renders under the sidebar:

Screenshot 2024-01-11 at 9 19 26 AM

I think this is just a fencepost error.

And second, the navbar is still scrollable even when the rest of the page content isn't:

Screen.Recording.2024-01-11.at.9.21.22.AM.mov

And I think through mild shenanigans we can fix both of these in the FixedLayout component:

const FixedLayoutDiv = styled.div`
  position: fixed;
  inset: /* top right bottom left */ calc(
      env(safe-area-inset-top, 0) + ${NavBarHeight} + 1px
    )
    env(safe-area-inset-right, 0) 0 env(safe-area-inset-left, 0);
`;

const FixedLayoutGlobal = createGlobalStyle`
  body {
    overscroll-behavior: none;
  }
`;

const FixedLayout = React.forwardRef<
  HTMLDivElement,
  React.HTMLAttributes<HTMLDivElement>
>((props, ref) => {
  return (
    <>
      <FixedLayoutGlobal />
      <FixedLayoutDiv ref={ref} {...props} />
    </>
  );
});

export default FixedLayout;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, neat -- if we do the FixedLayout shenanigans with disabling overscroll-behavior, we can get away with just always using the non-fixed navbar, even though it's the only part of the body whose content contributes to the flow (vs having fixed position)? That works for me!

The always-visible topbar is actually not the best use of space on most
pages:

* the topbar is easily found by scrolling back to the top of the page
* not having it take up 50 pixels of vertical space means 50 more rows
  for more-useful content like the puzzle list or guess queue
* having the top bar scroll away means we don't have to worry as much
  about having it be strongly-contrasting in color with other content
  that would slide under the fixed navbar.
* while some SPAs do have a more fixed layout, most other websites that
  have a navbar like this either do not make it sticky at all, or at
  least hide it when scrolling down

So let's just make the topbar be scrollable like most of the rest of the
web, and give a little more space for content.

We disable overscroll on pages where we use a fixed layout for the
majority of the page content and the navbar becomes the only part of the
page that uses default positioning, to avoid having just the navbar feel
wobbly.
@zarvox zarvox force-pushed the zarvox-scroll-navbar branch from 2efeabe to 4c968cc Compare January 11, 2024 16:32
@zarvox zarvox merged commit bb48e25 into main Jan 11, 2024
1 check passed
@zarvox zarvox deleted the zarvox-scroll-navbar branch January 11, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants