google / automotive-design-compose

Automotive Design for Compose is an extension to Jetpack Compose that allows every screen, component, and overlay of your Android App to be defined in Figma, and lets you see the latest changes to your Figma design in your app, immediately!
https://google.github.io/automotive-design-compose/
Apache License 2.0
118 stars 17 forks source link

More flexibility possible? #1368

Open StephanSchuster opened 4 months ago

StephanSchuster commented 4 months ago

I just started evaluating ADC. Really cool and useful. Thanks for all your work so far.

Do you think the following two features would be possible in future releases of ADC in order to increase its flexibility:

  1. Load offline docs also from resources (not only assets) --> enables Runtime Resource Overlays (RRO)
  2. Support dynamic docId and rootNode (e.g. via params) instead of generating/hardcoding them --> enables custom loading logic

Any feedback appreciated.

More details on (1): As far as I can see, this is where you currently try to load offline *.dcf files from the assets/figma dir: https://github.com/google/automotive-design-compose/blob/4dd69e283fcff3a0855c0bfdec2db951c972923c/designcompose/src/main/java/com/android/designcompose/DocServer.kt#L513

Before doing so, you could check in res/raw/ and then fall back to assets/figma. In consequence we could apply new *.dcf files via RRO which would be a great and mighty new feature.

More details on (2): Currently code like this

@DesignDoc(id = "XYZ...")
interface Test {

    @DesignComponent(node = "#MyRootNode")
    fun Main(
        ...
    )
}

results in the following generated code:

@Composable
final fun Main(...) {
    ...
    var nodeName = "#MyRootNode"
    ...
    val (docId, setDocId) = remember { mutableStateOf(DesignDocId("XYZ...", "")) }
    ....
}

See: https://github.com/google/automotive-design-compose/blob/4dd69e283fcff3a0855c0bfdec2db951c972923c/codegen/src/main/kotlin/com/android/designcompose/codegen/BuilderProcessor.kt#L642 https://github.com/google/automotive-design-compose/blob/4dd69e283fcff3a0855c0bfdec2db951c972923c/codegen/src/main/kotlin/com/android/designcompose/codegen/BuilderProcessor.kt#L779

I think if we could provide docId and rootNode dynamically, e.g. via optional params to the generated Composable, this would provide a hook for custom loading mechanisms. I other words, I would wish for a way to load a dedicated Figma doc/branch like the DesignSwitcher but from my own code.

StephanSchuster commented 4 months ago

Regarding (2): I guess I should simply not generate code via annotations but use the DesignDoc and CustomizationContext directly to achieve the requested dynamic/flexibility. In other words: write the generated code myself with the appropriate adaptions. Correct?

rylin8 commented 4 months ago

You can change the doc ID with the DesignDocOverride function. Example: https://github.com/google/automotive-design-compose/blob/c5af48beacc7afc0a7b936d70fe204ee15f49b2a/integration-tests/validation/src/main/java/com/android/designcompose/testapp/validation/examples/HelloBye.kt#L36

I'm not sure what your goal is exactly for changing the rootNode, but you can create different functions with different root nodes and call the appropriate one at runtime.

rylin8 commented 2 months ago

@StephanSchuster please let us know if the DesignDocOverride() function is sufficient.

StephanSchuster commented 1 week ago

Hi @rylin8, thanks for catching up and sorry that I did not reply since my initial question. I haven't had time since then to work with ADC. Now this hopefully changes.

You mentioned DesignDocOverride(). That's good to know, but not what I need. My overall goal is to have more control over the logic what to load from where and how to customize it - at runtime, dynamically, not via annotations. In my initial question I therefore asked for two related features.

I think for (2) I will be good to proceed as I mentioned in my second comment.

And for (1) I was hoping that you could add something like this into DocServer:

// ...
val assetManager = context.assets
try {
    @Suppress("DiscouragedApi")
    val resourceId = context.resources.getIdentifier("figma_${id.lowercase()}_dcf", "raw", context.packageName)
    val assetDoc = if (resourceId != 0) {
        // If file exists in res/raw, load from there
        context.resources.openRawResource(resourceId)
    } else {
        // Otherwise, load from assets (like done now)
        assetManager.open("figma/$id.dcf")
    }
    val decodedDoc = decodeDiskDoc(assetDoc, null, docId, Feedback)
    // ...
}
// ...

Background: files in assets cannot be overlayed via Runtime Resource Overlays. Only files in res can. With this I could provide a new Figma DCF file bundeled in a RRO at runtime and the app would change its appearance.

Maybe the order should be the other way round: assets first, and only if not found, check res. With that, overlaying would be prevented if a file in assets is provided.

Do you think there is a chance that you will add such functionality?

timothyfroehlich commented 1 week ago

@StephanSchuster we actually did earlier this week! #1778. We haven't added documentation for it, but the test that was added should show how to use it. It will be released as part of 0.32, and we cut 0.32.0-rc01 the other day.

StephanSchuster commented 1 week ago

@timothyfroehlich, @dipenpradhan : Yes, yes, yes!!! This is awesome. Thank you so much. I think this is a great addition.

I just tested it in rc-01. Works! One thing, however ...

The current comment for setRawResourceId(@RawRes resourceId: Int) says "Live updates do not work when this is set.". From what I see in code and from what I saw in my tests, this is not true. Live updates DO work and have priority.

The priorities as far as I can tell are:

  1. Live updates from Figma (if DesignSettings.enableLiveUpdates(this) is set)
  2. Offline file from res/raw (if DesignSettings.setRawResourceId(R.raw.xyz) is set)
  3. Offline file from assets
  4. Downloaded/cached files from app's filesDir

For me, this totally makes sense. Can you confirm that the above is true and you will stick with this?

StephanSchuster commented 1 week ago

Along the idea of "more flexibility", would it be possible to have a disableLiveUpdates()?

Background: We want/need to toggle various DesignSettings at runtime or rather call them with different values. While this works for setRawResourceId() and more or less for addFontFamily() (a "clear" method would be nice), we currently can only enableLiveUpdates() but never switch it off again.

rylin8 commented 1 week ago

The comment regarding live update not working is a bit misleading. If you enable live update the design switcher will show errors and you likely won't be able to see your app. We are aware of this issue and filed https://github.com/google/automotive-design-compose/issues/1802 to fix it.

StephanSchuster commented 1 week ago

@rylin8 : I think the current logic/priorities/implementation is fine as it is (see 1.-4. above). It totally makes sense. Also the single res/raw. I did not experience any errors. I would not change anything. I would just fix the comment.