Skip to content

Commit

Permalink
Closes #1056: Fix view finding for @DesignComponent functions on a co…
Browse files Browse the repository at this point in the history
…mponent set. (#1076)

Historically when a @DesignComponent is declared with a component set,
the component set name must match the property name in a @DesignVariant
parameter in order for the view matching algorithm to use the correct
variant at runtime. This has always been confusing and undocumented, so
this change allows the property name to differ from the component set
name. The view searching algorithm now checks at runtime of a node is
actually a variant of a component set, and uses that if so. Then, in
order to support TapCallback using the name of the component set,
populate variantParentName if we have the component set info to do so.

The VariantProperties test has been updated to test this scenario.
  • Loading branch information
rylin8 authored May 10, 2024
1 parent 5298baa commit d1a86cf
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,8 @@ internal fun DesignView(
}
)
val isRoot = LocalDesignIsRootContext.current.isRoot
val variantView = interactionState.rootNode(variantNodeQuery, document, isRoot)
val variantView =
interactionState.rootNode(variantNodeQuery, document, isRoot, customizations)
if (variantView != null) {
view = variantView
variantView.component_info.ifPresent { variantParentName = it.component_set_name }
Expand Down Expand Up @@ -1022,7 +1023,7 @@ internal fun DesignDocInternal(
else -> ""
}

val variantParentName =
var variantParentName =
when (rootNodeQuery) {
is NodeQuery.NodeVariant -> rootNodeQuery.field1
else -> ""
Expand Down Expand Up @@ -1061,7 +1062,7 @@ internal fun DesignDocInternal(

// Reset the isRendered flag, only set it back to true if the DesignView properly displays
if (doc != null) {
val startFrame = interactionState.rootNode(rootNodeQuery, doc, isRoot)
val startFrame = interactionState.rootNode(rootNodeQuery, doc, isRoot, customizations)
if (startFrame != null) {
LaunchedEffect(docId) { designComposeCallbacks?.docReadyCallback?.invoke(docId) }
CompositionLocalProvider(LocalDesignIsRootContext provides DesignIsRoot(false)) {
Expand All @@ -1085,6 +1086,14 @@ internal fun DesignDocInternal(
LocalParentLayoutInfo.current
}
DesignParentLayout(designViewParentLayout) {
if (startFrame.component_info.isPresent) {
// If this view is a variant but variantParentName was not set, set it here.
// This could happen if a node is replaced via component replacement with a
// variant of a component set.
val compInfo = startFrame.component_info.get()
if (variantParentName.isEmpty() && compInfo.component_set_name.isNotEmpty())
variantParentName = compInfo.component_set_name
}
DesignView(
modifier.semantics { sDocRenderStatus = docRenderStatus },
startFrame,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -527,15 +527,12 @@ private fun searchNodes(
q: NodeQuery,
nodes: Map<NodeQuery, View>,
parentViewMap: HashMap<String, HashMap<String, View>>,
variantPropertyMap: VariantPropertyMap
variantPropertyMap: VariantPropertyMap,
customizations: CustomizationContext? = null,
): View? {
nodes[q]?.let {
return it
}

if (q is NodeQuery.NodeVariant) {
val nodeName = q.field0
val componentSetName = q.field1
val findVariantView: (NodeQuery.NodeVariant) -> View? = { query ->
val nodeName = query.field0
val componentSetName = query.field1
val variantViewMap = parentViewMap[componentSetName]
if (variantViewMap != null) {
// Using the properties and variant names in the node name, try to find a view that
Expand All @@ -546,7 +543,38 @@ private fun searchNodes(
componentSetName,
variantViewMap
)
if (view != null) return view
view
} else {
null
}
}

if (q is NodeQuery.NodeName) {
// If the node query is by name, but there are variant properties set, check for a variant
// view with a node name generated from the variant properties and a component set name
// from the node query. This is a common scenario when a @DesignComponent function specifies
// a component set with @DesignVariant parameters to set which variant to use.
customizations?.variantProperties?.let { properties ->
if (properties.isNotEmpty()) {
val nodeNames: ArrayList<String> = arrayListOf()
properties.forEach { nodeNames.add("${it.key}=${it.value}") }
val nodeName = nodeNames.joinToString(",")
val view = findVariantView(NodeQuery.NodeVariant(nodeName, q.value))
view?.let { v ->
return v
}
}
}
}

nodes[q]?.let {
return it
}

if (q is NodeQuery.NodeVariant) {
val view = findVariantView(q)
view?.let { v ->
return v
}
}

Expand All @@ -570,7 +598,8 @@ private fun searchNodes(
internal fun InteractionState.rootNode(
initialNode: NodeQuery,
doc: DocContent,
isRoot: Boolean
isRoot: Boolean,
customizations: CustomizationContext,
): View? {
val findRootNode = {
if (isRoot) {
Expand All @@ -587,7 +616,13 @@ internal fun InteractionState.rootNode(
navOverlaySubscriptions.add(updateQuery)
onDispose { navOverlaySubscriptions.remove(updateQuery) }
}
return searchNodes(query, doc.c.document.views, doc.c.variantViewMap, doc.c.variantPropertyMap)
return searchNodes(
query,
doc.c.document.views,
doc.c.variantViewMap,
doc.c.variantPropertyMap,
customizations
)
}

@Composable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,12 @@ fun SquooshRoot(
interactionState.supportAnimations = true

val startFrame =
interactionState.rootNode(initialNode = rootNodeQuery, doc = doc, isRoot = isRoot)
interactionState.rootNode(
initialNode = rootNodeQuery,
doc = doc,
isRoot = isRoot,
customizationContext
)

if (startFrame == null) {
Log.d(TAG, "No start frame $docName / $incomingDocId")
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ interface VariantPropertiesTest {
@Design(node = "#Square4") square4: @Composable (ComponentReplacementContext) -> Unit,
@DesignVariant(property = "#bg1") bg1: Shape,
@DesignVariant(property = "#bg2") bg2: Shape,
@DesignVariant(property = "#SquareBorder") type: SquareBorder,
@DesignVariant(property = "BorderType") type: SquareBorder,
@DesignVariant(property = "#SquareColor") color: SquareColor,
@DesignVariant(property = "#comp1") comp1: CompType,
@DesignVariant(property = "#comp2") comp2: CompType,
Expand All @@ -79,7 +79,7 @@ interface VariantPropertiesTest {

@DesignComponent(node = "#SquareBorder")
fun Square(
@DesignVariant(property = "#SquareBorder") type: SquareBorder,
@DesignVariant(property = "BorderType") type: SquareBorder,
@DesignVariant(property = "#SquareColor") color: SquareColor,
@DesignVariant(property = "#SquareShadow") shadow: Shadow,
)
Expand Down

0 comments on commit d1a86cf

Please sign in to comment.