next: various header alignment/bugfixes by Rubiks-boy · Pull Request #15915 · thewca/worldcubeassociation.org

Rubiks-boy · 2026-10-04T20:36:44Z

fixes bug where empty search results content flashes when you click into the search bar
various alignment fixes in the header

before	after



(0.5x speed)

Rubiks-boy · 2026-10-04T20:45:49Z

  return (
    <Combobox.Root
      collection={collection}
-      // We search server-side, so disable Combobox's built-in filtering.


this comment seemed totally unrelated... I'm not sure why it's here. I looked thru the git blame to see if it previously applied to a different prop, but couldn't track it down.

I tested search behavior after, and everything seemed to still work fine/I didn't notice any weird filtering.

You're probably right, I remember that Finn touched the search bar multiple times over the span of several PRs. Might be as easy as a "merge conflict whoopsie".

CC @FinnIckler for double-confirmation

gregorbg

I really love the attention to detail! It comes at a high price though, because you're adding a lot of manual padding all over the place.

It's not strictly a violation of our style guide, because you are referencing proper Chakra tokens and not "wild" hard-coded pixel values. But the sheer amount of Chakra tokens that you need to put in here and there and beyond, just makes me feel a tiny bit uncomfortable.

Instead of focussing on the individual components and giving every list item a padding, maybe could it make sense to "zoom out" layout-wise and apply the inner margins to the list's container instead?
I am not referencing a specific list in a specific file of your PR, rather the term "list" is just an arbitrary example of the container-and-items setting.

Unfortunately I cannot put my finger on a exact location, without digging very deep into your changeset. But just looking over the diff, I really have a gut feeling there's some "chicken and egg" thing going on where you're adjusting too much on the "bullet point" level and not enough on the "item container" level

gregorbg · 2026-10-05T16:01:12Z

+        <HStack ps={3} pe={1.5} py={3} justifyContent="space-between">
          <HStack>
-            {!LIVE_RESULT_BETA && <WCALogo />}
+            {!LIVE_RESULT_BETA && <WCALogo mr={1} />}


Suggested change

{!LIVE_RESULT_BETA && <WCALogo mr={1} />}

{!LIVE_RESULT_BETA && <WCALogo me={1} />}

heh, valid given my other PR 😭

gregorbg · 2026-10-05T16:02:18Z

  return (
    <Combobox.Root
      collection={collection}
-      // We search server-side, so disable Combobox's built-in filtering.


You're probably right, I remember that Finn touched the search bar multiple times over the span of several PRs. Might be as easy as a "merge conflict whoopsie".

gregorbg · 2026-10-05T16:02:31Z

  return (
    <Combobox.Root
      collection={collection}
-      // We search server-side, so disable Combobox's built-in filtering.


CC @FinnIckler for double-confirmation

Rubiks-boy · 2026-10-06T04:34:57Z

@gregorbg I think I got something a little bit cleaner.

Semantically, I want to align things based on the button's text instead of the button's border box, and have the container's padding dictate how far we indent the button's text. Other component libraries accomplish this by applying negative margins to ghost buttons (e.g. Radix's button). But Chakra doesn't!

I used this negative margin idea in next-frontend/src/components/MobileNav.tsx. This let us remove quite a few of our extra overrides (e.g. extra margins on separators) and overall let the container's padding dictate how far indented the text it.

gregorbg · 2026-10-06T07:15:01Z

    base: {
+      // Ensure hover/focus outlines don't get clipped when we hide overflow
+      content: {
+        mx: "-3",
+        px: "3",
+      },


Adjusting this on the base level feels dangerous, because it affects every Collapsible anywhere in the code.

This may be the solution for all Collapsibles that we have thus far, but will it also work for all collapsibles in the future?

If you are confident that Yes, this will work, then is it something that should be reported to Chakra upstream? (Because if you really want to consciously apply it to any and all Collapsibles ever out there, then the original Chakra recipe might be wrong/off)

Good catch! Yeah, this is actually really dangerous - I forgot that this affects width: 100% calculations, which turns out would have caused a small regression on /competitions.

at the risk of slightly duplicating some the props, I'm adding these props inline where they're actually needed

Mh, this actually amounts to a pretty big and delicious copy/pasta. Do you know that Chakra supports boolean recipes, where you declare a variant with true as the string key, and the framework flips it into a boolean for you?

The point here is not (only) reusability fetishism, but also the fact that the recipe declaration grants you a very "natural" place to write code comments about why this recipe exists in the first place and how these negative values came to be

gregorbg · 2026-10-06T07:19:31Z

    <Button
-      asChild
-      variant="ghost"
+      variant={active ? "subtle" : "ghost"}


Hmmm, this active property is only used actively (no pun intended) once throughout the whole PR. It's in the "choose your language" dropdown menu to visually mark a button as "you are selected".

But as you can imagine, changing the variant of a button just to mark "is this option selected" is not really the intended use-case for variants. Perhaps something like a vertical Segmented Control is more suitable here?

I am nit-picking this because without the active toggle boolean, this whole thing could simply be turned into a recipe under theme.ts. So it might be worth investigating an alternative component for the locale picker for 10-15 minutes, and get the easy win of a mobileNav variant under the button recipe.

👀 oops, this active prop was sort of slop that leaked in, didn't catch it 🤦

I think going segmented control could cause some weirdness with keyboard nav: if you use arrow keys, it immediately selects the new option, leading to the page reloading. For now, going to revert back to what's on main (which passes the variant inline), and add an extra aria-current attribute

Lmk if you'd prefer turning into a recipe (and/or if you have a good idea for a name :P). I'm not sure how applicable it is outside of this one mobile nav situation, so not sure if that affects naming and/or whether to turn into a recipe.

I think I'm coming around to your point that it doesn't need to be a recipe because this specific button only applies to one specific situation.

If someone finds this in six months though, and realizes "Oh this is just what I needed!" then there's a risk of reusing this component when it really should become a recipe.

Makes sense! Maybe we can revisit if (or more likely when) that happens?

(I'm pretty sure there's some piece even more generalizable here, probably either:

a boolean prop on ghost buttons that cancels out padding with negative margins

some buttonPadding="xs"|"sm"|"md"|... prop that lets us use a different button size's padding preset

but I might need to see a few more examples to know for sure :))

github-actions Bot assigned Rubiks-boy Oct 4, 2026

Rubiks-boy commented Oct 4, 2026

View reviewed changes

Rubiks-boy force-pushed the next-header-fixes branch from 5e4e18a to 61956e6 Compare October 4, 2026 21:01

Rubiks-boy marked this pull request as ready for review October 4, 2026 21:04

gregorbg reviewed Oct 5, 2026

View reviewed changes

Rubiks-boy force-pushed the next-header-fixes branch from 4900f8d to 94dc4a4 Compare October 6, 2026 04:23

gregorbg requested changes Oct 6, 2026

View reviewed changes

Rubiks-boy added 7 commits October 6, 2026 00:26

Prevent empty search dropdown from opening on click

c9615f8

decrease search padding

c66a2d5

Decrease sign-in and language picker padding

d632d12

various alignment fixes

1009e93

Decrease navbar avatar size to 28px

daab278

Keep main navbar avatar at 40px

ea2e88a

Align mobile navigation buttons and prevent clipped outlines

9cc1a97

Rubiks-boy force-pushed the next-header-fixes branch from 94dc4a4 to 9cc1a97 Compare October 6, 2026 07:28

Rubiks-boy added 2 commits October 6, 2026 00:38

oops cleanup

1c13837

Scope collapsible clipping fix to mobile navigation

4935cfd

	{!LIVE_RESULT_BETA && <WCALogo mr={1} />}
	{!LIVE_RESULT_BETA && <WCALogo me={1} />}

Conversation

Rubiks-boy commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

gregorbg 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!

Choose a reason for hiding this comment

Uh oh!

Rubiks-boy commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

Rubiks-boy Oct 6, 2026 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Rubiks-boy commented Oct 4, 2026 •

edited

Loading

Rubiks-boy commented Oct 6, 2026 •

edited

Loading

Rubiks-boy Oct 6, 2026 •

edited

Loading