excalidraw icon indicating copy to clipboard operation
excalidraw copied to clipboard

feat: refactor Sidebar into standalone reusable component

Open dwelle opened this issue 2 years ago • 3 comments

Refactored editor sidebar into its own component so that host apps can use it for their own needs.

Last commit contains a minimal working example. Will remove before merge.

Exported components, and added APIs:

  • Components:
    • <Sidebar onClose onDock docked />
    • <Sidebar.Header />
  • <Excalidraw /> props:
    • children
    • onMenuToggle(type: string, isOpen: boolean)

TODOS

  • [ ] clickOutside triggering on padding coz we bind it on LibraryMenuWrapper container, and the padding is on its container (separately on Sidebar and library floating menu). Will require some CSS/DOM shufflling. Not a huge priority, may leave for another PR.
  • [ ] update Readme
  • [ ] update Changelog
  • [ ] update example app
  • [ ] add codebase docs

Notes

  • Several solutions are hacky/ overcomplicating stuff internally in order to make it better for host apps. I wanted to investigate how we could do things and if what I had in mind works or not, with respect to oncoming Excalidraw API rewrite. We may decide it's not worth it/ realize it doesn't scale, and change the API - later.

  • Notifying host apps that an editor sidebar has toggled so that they can act on it (close theirs) I'm doing via a new API called onMenuToggle(type: "library", isOpen: boolean), which is meant to be more generic and later notify of other types of menus being opened (such as export dialog and such, so that host apps can react on those as well).

    It was also weird adding a onSidebarToggle since we're calling it also when the floating library menu is toggled, not just the sidebar library menu (which I think is desired as we don't want both a (host) sidebar and a floating menu to co-exist at once) — though we may want to remove the floating menu altogether in favor of sidebar-only solution, see below.

    And adding a onLibraryToggle was too specific.

  • We should decide whether the custom sidebar should also behave identically to library menu on mobile, and turn into a floating menu.

    Alternatively, we may want to remove the floating library menu in favor of sidebar-only. When the sidebar wouldn't fit in any usable way, we can fullscreen it, same as we do with e.g. comment thread popups in Excalidraw+.

  • I haven't introduced <Sidebar.Content/> nor <Sidebar.Footer/> because it's not needed and host apps (same as Excalidraw) may want to have more control. We may introduce later if it makes sense.

solutions / problems

  1. Render <Sidebar/> as children.

    Explanation: Honestly, this is likely just an aesthetic preference.

    API:

    <Excalidraw>
        <Sidebar onClose>
            {/* custom content */}
        </Sideber>
    </Excalidraw>
    

    Solution: Added support for props.children on <Excalidraw/> and render them as a sibling of <LayerUI/>. We may put it inside <LayerUI/> later, but for now it's fine not complicating it (prop drilling for desktop and mobile menu). It's also possible that we may support non-UI children (behavior plugins etc.), so rendering all children to <LayerUI/> may not make sense.

    Considerations: With <Sidebar/> it's not very important where it's rendered as it's absolutely positioned. ~~With future components, it's likely we won't be able to use props.children as we won't be able to render everything as siblings, inconsiderately.~~ Actually, we may figure out some trickery using Portals.

  2. Not forcing hosts to always render <Sidebar.Header/> themselves.

    Explanation: We want to make using the Sidebar as easy as possible, so that people don't have to read through documentation too much in order to figure out what they need to do to make it work — especially when they're fine with the defaults.

    API: if you don't need a custom <Sidebar.Header/>, you'd just do:

    <Sidebar>
        {/* your content */}
    </Sidebar>
    

    And we will render our default one.

    Solution:

    Internally, the Sidebar component wraps the content in React Context, always renders the default <Sidebar.Header/> with a __isFallback set to true. When the host renders the Sidebar.Header by itself (where __isFallback isn't set), it updates the context's isRenderedUpstream to true to indicate we shouldn't render the fallback Header (the default).

    For this, I've created a withUpstreamOverride(component) HOC since I assume we'll need this pattern a lot — provided we can afford it in terms of performance/memory.

  3. Rendering one sidebar at a time.

    Explanation: Both the editor and the host app will have sidebars, and we must render only one at a time. Further, we want to give precedence to the host app's sidebar.

    API: If the host app renders the sidebar (i.e. it's in DOM), never render the editor's sidebar. NOTE: currently, we don't handle the case where the host app renders two or more sidebars, and leave it to the host app to handle by itself.

    Solution: We keep track of the number of host sidebars being rendered (though it should be at most 1), and skip rendering editor sidebar if > 0. To do that, all internal editor sidebars must set __isInternal prop to true.

    We don't want to force host apps to wrap their apps in an Excalidraw React Context (for now, though we may in the future), so we'll use a Jotai atom instead. (Also see 4).

    Alternative solutions: If we use a render prop, then we can just null check it.

  4. Offsetting UI when any sidebar open & docked.

    Explanation: when either the editor or host sidebar is open and docked, we need to offset the UI. In other words, we need to make the <Sidebar/> let the <LayerUI/> know whether it should offset itself (and since we need to account for host sidebars, we can't read from appState).

    API: Whenever <Sidebar docked/> is true.

    Solution: We use Jotai atom counter which we increment/decrement based on rendered Sidebars that have docked set to true. We use the same atom as for (3).

  5. Notifying Sidebar consumers of state changes.

    Explanation: we need to handle onClose, onDock etc. If we consider onClose, currently there's only one way to close the sidebar: using the close button inside Sidebar.Header. Later, we may support keyboard (Esc) as well.

    API: The <Sidebar/> doesn't necessarily render the <Sidebar.Header>, and host apps may not render it either — as such, we can't put an onClose prop on the Header (which would also not be semantic in case of closing it via keyboard), so we must add it on the <Sidebar/> itself.

    Solution: We use a React Context that contains all the necessary callbacks and state that the children we can't easily pass the props to can read from.

dwelle avatar Sep 04 '22 21:09 dwelle

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
excalidraw ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Oct 17, 2022 at 10:10AM (UTC)
excalidraw-package-example ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Oct 17, 2022 at 10:10AM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
docs ⬜️ Ignored (Inspect) Oct 17, 2022 at 10:10AM (UTC)

vercel[bot] avatar Sep 04 '22 21:09 vercel[bot]

@excalibot trigger release

dwelle avatar Sep 05 '22 20:09 dwelle

@dwelle Preview version has been shipped :rocket: You can use @excalidraw/[email protected] for testing!

excalibot avatar Sep 05 '22 20:09 excalibot