fix: Apply theme to only to active excalidraw component

[ad1992]

Make sure theme is only applied to the active Excalidraw component when multiple excalidraw rendered

Exposed a hook that gives access to the current instance, this will be useful when focusing the current instance as well(ex after dialog close, we can expose focus function as well for the same)
Since collab dialog is not in the hierarchy of App.tsx hence had to add a new prop theme so it can be used to add a theme to custom dialogs as well.

Try here

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/excalidraw/excalidraw/28wp4vKmj8rM1jALNfe37YEJXtx8
✅ Preview: https://excalidraw-git-aakansha-theme-excalidraw.vercel.app

[dwelle]

Conceptually looks fine. But we should name it ExcalidrawContainerContext since it's not an instance. Also, why not store the container value directly instead of wrapping it in {current}?

[ad1992]

Conceptually looks fine. But we should name it ExcalidrawContainerContext since it's not an instance. Also, why not store the container value directly instead of wrapping it in {current}?

I am thinking will also expose the focusContainer method when we start storing active element hence keeping key value pairs

[dwelle]

I am thinking will also expose the focusContainer method when we start storing active element hence keeping key value pairs

That will mess up with memoization. I'd make it be just the container and if we want to factor out the focusing, we can make it an util method again (but I don't see much reason since it'll be just context?.focus())

[ad1992]

I am thinking will also expose the focusContainer method when we start storing active element hence keeping key value pairs

That will mess up with memoization. I'd make it be just the container and if we want to factor out the focusing, we can make it an util method again (but I don't see much reason since it'll be just context?.focus())

I don't think apart from the focus I was planning to expose anything else and one can directly call context.?.focus so should be fine. We can tweak it later if needed.

[dwelle]

if we don't want to use theme only (since it's optional), it should nevertheless take precedence since it's explicit, so let's swap the order of checks

[ad1992]

unclear what you mean, theme is given less precedence since its optional over excal container so why we want to check theme first ?

[dwelle]

if it's supplied and for some reason isn't matching the container, we should prefer the explicit value. Is there actually a reason why we're supplying it at all? Aren't Modals always part of the component tree? EDIT: forgot about Collab

[dwelle]

But I suppose since this isn't public API it doesn't matter, and actually is preferable UX-wise. It'll just mean the app might have slight bugs without us knowing.

[ad1992]

Yes, this isn't public API, if in the future we expose Dialog then it can be helpful for the host but I don't think we will be doing that as the host will always have its own dialogs and since its for excal app itself so I have given preference to excal container over theme.