Conflicting declarations on two components: Full-width layout and header search

It seems both components re-arrange the .contents div in the header.

The structure with the full-width layout:
image

Structure with header-search:
image

So I can’t use them together. But not sure where to best fix this?

3 Likes

I ran into this exact problem with these two components last week… it’s a little tricky because I intended the full-width component to be a temporary experiment, but we have nothing on the roadmap to integrate it by default so it’s sticking around longer than I expected.

The full-width component isn’t ideal because it requires changing the header template (the only way I could overcome some layout issues).

At a glance… I don’t think the template override in the header search component is necessary, so I can take a look at moving that to a widget decorator, which would avoid the problem.

4 Likes

What I tried as a temp. fix is not changing the header-contents widget on the full-width component. So the sidebar-toggle and the title are not grouped under __toggle-and-logo. And then just arrange both in the toggle grid-area. I didn’t see layout issues with this so far. But I’m probably missing sth?

I think it’s very popular. I have three current customization projects and all opted for full-width. That’s also why I posted this, I’d prefer to not tweak official components to achieve what seems to be a common choice.

2 Likes

If I recall correctly, the extra wrapper makes it easier to get the title aligned with the topic content because I can set the combined .header-contents__toggle-and-logo width to var(--d-sidebar-width), and it’s the same width as the sidebar regardless of the content.

Screenshot 2023-06-26 at 4.52.20 PM

Without the extra wrapper, the layout is workable… but with two grid columns I can’t rely on a single width for both.

Screenshot 2023-06-26 at 4.54.13 PM

I need to assume the sidebar toggle will always be some static width, and then calc the max logo width based on that. That works, but I recall it being more fragile… I haven’t looked at this for a while, but maybe it’s worth another attempt :thinking:

Back to this… I can see why it was done with an override. Without it you have to decorate the title or header-icons widget, so you end up adding content inside .title or .panel, which makes center-alignment of the search bar more difficult… and requires some CSS that makes the layout more fragile and makes compatibility with other header components harder. Ideally the search bar content should be outside of these divs, but there’s nothing to hook into to do that.

We can now add a plugin outlet to widgets, so that could help…

This would allow content to be added before the .panel div, rather than inside it with decorateWidget. In this case the template override could be removed from the header-search component, and a new connector can be added to:

javascripts/discourse/connectors/before-header-panel

which could contain

<MountWidget @widget="search-banner" />

Adding a plugin outlet to a widget so we could add a widget to a plugin outlet seems a little convoluted though… @david/@cvx do you know if this would be bad for performance or cause any other issues ?

1 Like

btw, here’s what I tried as a fix on the full-width component: Comparing discourse:main...nolosb:header-contents · discourse/discourse-full-width-component · GitHub

That is:

  • use the default template layout without additional wrapping div around toggle and title logo
  • match items to grid areas

However, I see that the title-logo switches back to the small logo when topic titles are shown on the header. This also happens here on meta on the full-width layout. I don’t really understand which template arguments to use here to always show the full logo.

Oh I see, you put them both in the same grid area and apply a margin to the logo… that seems like a reasonable compromise!

that’s why the home-logo is reopened here:

If the sidebar isn’t shown, it uses the default logo logic to switch between big/small… if the sidebar is shown, it always returns the big logo.

1 Like

It might be a little slower, but I’m not too concerned in this case because there’s only one instance of the plugin outlet on the page (as opposed to, for example, a topic-list-item plugin outlet which would be rendered 25+ times).

Adding an outlet there sounds great to me :white_check_mark:

1 Like

ok, so I’ve updated both components to avoid template overrides —

So now they should work together :rocket: thanks for the suggestions @nolo!

3 Likes