excalidraw
excalidraw copied to clipboard
feat: refactor Sidebar into standalone reusable component
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
-
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 useprops.children
as we won't be able to render everything as siblings, inconsiderately.~~ Actually, we may figure out some trickery using Portals. -
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 theSidebar.Header
by itself (where__isFallback
isn't set), it updates the context'sisRenderedUpstream
totrue
to indicate we shouldn't render the fallbackHeader
(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. -
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 totrue
.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.
-
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 fromappState
).API: Whenever
<Sidebar docked/>
is true.Solution: We use Jotai atom counter which we increment/decrement based on rendered Sidebars that have
docked
set totrue
. We use the same atom as for (3). -
Notifying
Sidebar
consumers of state changes.Explanation: we need to handle
onClose
,onDock
etc. If we consideronClose
, currently there's only one way to close the sidebar: using the close button insideSidebar.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 anonClose
prop on theHeader
(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.
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) |
@excalibot trigger release
@dwelle Preview version has been shipped :rocket: You can use @excalidraw/[email protected] for testing!