Rewriting Sidebar API

[dwelle]

• [ ] support for sidebar tabs
• [ ] migrate to Excalidraw child (remove props.renderSidebar)
• [ ] rewrite state handling

component proposal

<Sidebar>
  <Sidebar.Trigger>Custom togle</Sidebar.Trigger>
  <Sidebar.Tabs>
    <Sidebar.TabContent name="library">
      {/* ... */}
    </Sidebar.TabContent>
    <Sidebar.TabContent name="comments">
      {/* ... */}
    </Sidebar.TabContent>
    <Sidebar.TabContent name="frames">
      {/* ... */}
    </Sidebar.TabContent>
    <Sidebar.TabList>
      <Sidebar.TabTrigger name="library" />
      <Sidebar.TabTrigger name="comments" />
      <Sidebar.TabTrigger name="frames" />
    </Sidebar.TabList>
  </Sidebar.Tabs>
</Sidebar>

(component naming & structure TBD)

• we'd be passing the component always (same as props.renderSidebar()), and the editor would render it based on state
• appState.openSidebar can stay
• change appState.openSidebar state to string | null (could be narrowed down to host-enabled tabs if we figure out how to do it)
• null means closed sidebar
• string would open sidebar with the tab it's set to. E.g. library or comments
• we either remove toggleMenu for now, or we change its signature to { type: "sidebar", tab: string }

[maielo]

Looks great!

Reminds me radix.ui tabs :)

[dwelle]

Notes on the updated API.

• The sidebar trigger is now a <SidebarTrigger> instead of <Sidebar.Trigger>, because now you should be rendering it outside of the sidebar as it's not tied to a single one. It's also not conceptually inside the sidebar. And you may wanna render your custom sidebar conditionally for perf, but want to render the trigger always:

  <Excalidraw>
    <SidebarTrigger>Custom trigger</SidebarTrigger>
    <Sidebar>
      {/* ... */}
    </Sidebar>
  </Excalidraw>
  

• We will support rendering multiple custom sidebars (as siblings). We get this for free with current implementation, and it will allow people to have multiple sidebars without having to use tabs if they don't want to:

  <Excalidraw>
      <Sidebar name="custom">
        custom 1
      </Sidebar>
      <Sidebar name="custom2">
        custom 2
      </Sidebar>
  </Excalidraw>
  

• Each <Sidebar/> has to have a name prop which we use to identify it and render it based on state.

• The sidebar tabs are optional, so people can still render content without tabs into <Sidebar.Content/>.

[maielo]

Shouldn;t name of <SidebarTrigger> be <Sidebar.Trigger> to align in with rest ? (i know it's outside, but namespace wise it should be same IMHO)

[dwelle]

Shouldn;t name of <SidebarTrigger> be <Sidebar.Trigger> to align in with rest ? (i know it's outside, but namespace wise it should be same IMHO)

I'm afraid <Sidebar.Trigger> will make it seem you should use it inside <Sidebar> and that it belongs to a specific <Sidebar> component, whereas it's replacing the editor's sidebar (currently library) toggle.

And we may actually support multiple triggers (which would then be <Sidebar.Trigger>) which you would be able to render anywhere. E.g. on Plus the comment button could be a <Sidebar.Trigger>.

So perhaps we should rename it to <SidebarMainToggle>. Yeah, it's not great.

---

One thing I just realized is that we need to add support for rendering custom sidebar and merging the library sidebar into it as one of the tabs.

[dwelle]

API for sidebar headers

When rendering multiple tabs, you may want to have a different sidebar header for each tab. For example, a sidebar that contains library and comments tabs will have different headers for each.

There are two ways to do this:

• API variant 1

  Nest multiple headers as direct children of <Sidebar> (as we do currently), where each header will have tab prop binding it to a specific tab.

  <Sidebar name="custom" ref={ref}>
    <Sidebar.Header tab="one">Tab one custom header</Sidebar.Header>
    <Sidebar.Header tab="two">Tab two custom header</Sidebar.Header>
    <h2>Custom sidebar</h2>
    <Sidebar.Tabs defaultTab="one">
      <Sidebar.TabContent value="one">
        Tab one content
      </Sidebar.TabContent>
      <Sidebar.TabContent value="two">
        Tab two content
      </Sidebar.TabContent>
      <Sidebar.List>
        <Sidebar.TabTrigger value="one">One</Sidebar.TabTrigger>
        <Sidebar.TabTrigger value="two">Two</Sidebar.TabTrigger>
      </Sidebar.List>
    </Sidebar.Tabs>
  </Sidebar>
  

• API variant 2

  Nest each header inside the respective <Sidebar.TabContent>.

  Pros:

  • No need for binding through tab props, and the headers will be nested where they belong. This will also allow for having each tab as its own component with its own state, especially if the header depends on that state.
  • Actually should be easier to implement on our side.

  Cons:

  • slightly less semantic in that you will render it inside TabContent as sibling of its content, but it will be rendered outside the tab DOM. But, that's mostly an implementation detail. But, it will force Header position (on top) because we will render using tunnels. Currently, we allow rendering the header wherever you want. But I think it's fine.

    You will also be able to render the header outside of the TabContent if you want just one header per whole sidebar (all tabs).

  <Sidebar name="custom" ref={ref}>
    <h2>Custom sidebar</h2>
    <Sidebar.Tabs defaultTab="one">
      <Sidebar.TabContent value="one">
        <Sidebar.Header>Tab one custom header</Sidebar.Header>
        Tab one content
      </Sidebar.TabContent>
      <Sidebar.TabContent value="two">
        <Sidebar.Header>Tab two custom header</Sidebar.Header>
        Tab two content
      </Sidebar.TabContent>
      <Sidebar.List>
        <Sidebar.TabTrigger value="one">One</Sidebar.TabTrigger>
        <Sidebar.TabTrigger value="two">Two</Sidebar.TabTrigger>
      </Sidebar.List>
    </Sidebar.Tabs>
  </Sidebar>
  

[dwelle]

I'm also thinking of nuking the "render default header if no custom one supplied". Will remove tons of complexity. In other components we don't do that either, and make you render your content instead (e.g. MainMenu).

EDIT: the preventing 2+ headers at the same time we may actually keep for cases like adding custom tabs into default sidebar without having to render common header on top.

[ad1992]

API for sidebar headers

When rendering multiple tabs, you may want to have a different sidebar header for each tab. For example, a sidebar that contains library and comments tabs will have different headers for each.

There are two ways to do this:

• API variant 1 Nest multiple headers as direct children of <Sidebar> (as we do currently), where each header will have tab prop binding it to a specific tab.

  <Sidebar name="custom" ref={ref}>
    <Sidebar.Header tab="one">Tab one custom header</Sidebar.Header>
    <Sidebar.Header tab="two">Tab two custom header</Sidebar.Header>
    <h2>Custom sidebar</h2>
    <Sidebar.Tabs defaultTab="one">
      <Sidebar.TabContent value="one">
        Tab one content
      </Sidebar.TabContent>
      <Sidebar.TabContent value="two">
        Tab two content
      </Sidebar.TabContent>
      <Sidebar.List>
        <Sidebar.TabTrigger value="one">One</Sidebar.TabTrigger>
        <Sidebar.TabTrigger value="two">Two</Sidebar.TabTrigger>
      </Sidebar.List>
    </Sidebar.Tabs>
  </Sidebar>
  

• API variant 2 Nest each header inside the respective <Sidebar.TabContent>. Pros:

  • No need for binding through tab props, and the headers will be nested where they belong. This will also allow for having each tab as its own component with its own state, especially if the header depends on that state.
  • Actually should be easier to implement on our side.

  Cons:

  • slightly less semantic in that you will render it inside TabContent as sibling of its content, but it will be rendered outside the tab DOM. But, that's mostly an implementation detail. But, it will force Header position (on top) because we will render using tunnels. Currently, we allow rendering the header wherever you want. But I think it's fine. You will also be able to render the header outside of the TabContent if you want just one header per whole sidebar (all tabs).

  <Sidebar name="custom" ref={ref}>
    <h2>Custom sidebar</h2>
    <Sidebar.Tabs defaultTab="one">
      <Sidebar.TabContent value="one">
        <Sidebar.Header>Tab one custom header</Sidebar.Header>
        Tab one content
      </Sidebar.TabContent>
      <Sidebar.TabContent value="two">
        <Sidebar.Header>Tab two custom header</Sidebar.Header>
        Tab two content
      </Sidebar.TabContent>
      <Sidebar.List>
        <Sidebar.TabTrigger value="one">One</Sidebar.TabTrigger>
        <Sidebar.TabTrigger value="two">Two</Sidebar.TabTrigger>
      </Sidebar.List>
    </Sidebar.Tabs>
  </Sidebar>
  

What kind of tabs are you talking about inside Sidebar ? Is it the normal tabs for switching layout ? In that case we can have separate tabs components and use that inside Sidebar

[ad1992]

render default header

I didn't get you, you mean render default header if not passed ?

[dwelle]

What kind of tabs are you talking about inside Sidebar ? Is it the normal tabs for switching layout ? In that case we can have separate tabs components and use that inside Sidebar

Yes, that's what this issue is about (we'll use https://www.radix-ui.com/docs/primitives/components/tabs underneath). But the comment you're quoting is about multiple headers, one for each tab.

render default header

I didn't get you, you mean render default header if not passed ?

yep.

[dwelle]

closed via https://github.com/excalidraw/excalidraw/pull/5663 and https://github.com/excalidraw/excalidraw/pull/6213