Proposal: Make the Mount component more versatile and add a component for showing and hiding #108
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make the Mount component more versatile and add a component for showing and hiding
This is a proposal to make two primary changes to our mounting components:
Mount
component more versatile.Note that I have not yet written any documentation for these proposed changes, but if others agree on these changes, then I will do so before this PR is merged.
Make the
Mount
component more versatileCurrently, the
Mount
component only accepts facets of typeFacet<boolean | undefined>
. This PR would enable the components to instead accept a more diverse number of facet types:Facet<T | null | undefined>
.The motivation for this change is to reduce code bloat while also slightly boosting performance by cutting down on redundant
useFacetMap
's. Note that one would still need to use auseFacetMap
for more advanced conditionals.In our current API, one often has to create a
Facet<boolean>
to conditionally mount elements.With this updated API, this wouldn't be necessary for simple conditionals.
Other changes to the
Mount
componentThis PR also introduces two other minor changes to the component.
not
prop.condition
prop tois
.Introduce a
not
propTo make full use of the new versatility, one can now negate the condition.
Rename the
condition
prop tois
I renamed this variable to follow the naming convention introduced by the
when
prop. The idea is that you write a sentence to use the component: mount when my facet is true. Arguably, thenot
prop could be namedisNot
to more closely follow this format, but I thoughtnot
sounded cleaner and still follow the rule fairly well.Add a performant component to show and hide JSX elements
For scenarios where we really need to push the performance, it is often beneficial to show and hide elements rather than mounting and unmounting them (and without using
useFacetUnwrap
), and I think this should be a part of our core mounting framework to standardize the pattern. Note that there are complications to leaving elements mounted (such as memory usage and screen narration), but it is nonetheless a viable option in many scenarios.For example, images can often be faster to show and hide than they are to mount and unmount. With our current API, this would require quite a bit of bloat.
I'm proposing a component to standardize this pattern. I am tentatively calling it
Show
, but as it awkwardly needs to wrap its children in an HTML element (afast-div
), I think that maybe it should be called something more explicit, such asShowFastDiv
. Would love feedback on the name and its API.The new API would allow one to write the above code with less bloat.
It accepts all props that
Mount
would, and all props thatFastDiv
allows for besidesstyle
. It is possible to add support for thestyle
prop, but I believe it would slightly reduce the performance of the component to handle it. Individual style can instead be passed down with aclassname
.Remaining work
The
Show
component wraps its children in adiv
, and so it uses@react-facet/dom-fiber
. I'm not sure if theShow
component should really live among our other mounting components, or if it should actually live indom-fiber
, perhaps as an alternative tofast-div
. In that case, it might use a different naming convention.This package conundrum is also why the PR has failing checks.