Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avocado removes @string paths #27

Open
FrancoisBlavoet opened this issue Jan 22, 2018 · 3 comments
Open

Avocado removes @string paths #27

FrancoisBlavoet opened this issue Jan 22, 2018 · 3 comments

Comments

@FrancoisBlavoet
Copy link

I ran Avocado on this xml :

<vector xmlns:android="http://schemas.android.com/apk/res/android"
        xmlns:tools="http://schemas.android.com/tools"
        android:width="108dp"
        android:height="108dp"
        android:viewportHeight="108.0"
        android:viewportWidth="108.0">
    <group
        android:translateX="23"
        android:translateY="23">
        <path
            android:fillColor="#2A7618"
            android:fillType="nonZero"
            android:pathData="@string/ic__core_path_carrot"
            tools:targetApi="n"/>
        <path
            android:fillColor="#2A7618"
            android:fillType="nonZero"
            android:pathData="@string/ic__core_path_carrot_leaves"
            tools:targetApi="n"/>
    </group>
</vector>

Here is the output I got :

<vector xmlns:android="http://schemas.android.com/apk/res/android"
        android:width="108dp"
        android:height="108dp"
        android:viewportHeight="108.0"
        android:viewportWidth="108.0"/>

In all fairness, it is smaller, but I did lose a bit of info there :p

@alexjlockwood
Copy link
Owner

Oops, good catch. The tool doesn't know how to optimize string resources yet, but until then it should just ignore them instead of deleting them entirely.

@FrancoisBlavoet
Copy link
Author

haha, no pb
I just wanted to see if it was going to bake this translation into the path for me and it does 🎉 (as long as it is not a string reference anyway, but that's fair)

@alexjlockwood
Copy link
Owner

For future reference, I need to update the regex string in the removeHiddenElems plugin to detect string resources: https://github.com/alexjlockwood/avocado/blob/master/src/plugins/removeHiddenElems.ts#L4

I also need to update the mergePaths plugin so that it doesn't attempt to concatenate paths that reference string resources: https://github.com/alexjlockwood/avocado/blob/master/src/plugins/mergePaths.ts

Probably worth double checking the convertPathData plugin as well to make sure it isn't making any assumptions about the validity of the path data attribute value: https://github.com/alexjlockwood/avocado/blob/master/src/plugins/convertPathData.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants