ionic-team / trapeze

The mobile project configuration toolbox. Manage native iOS, Android, Ionic/Capacitor, React Native, and Flutter apps through a simple YAML format.
https://trapeze.dev
Other
328 stars 40 forks source link

Bug: appName has no effect (android, capacitor V5) #191

Closed tobiasmuecksch closed 1 year ago

tobiasmuecksch commented 1 year ago

Description

This issue is explicitly about android. I'm trying to manipulate the app name with trapeze. According to the docs appName is the way to go. Therefore I've set an appName like this:

vars:
  APP_NAME:
    default: cap5 Test App
  PACKAGE_NAME_ANDROID:
    default: de.tobiasmuecksch.cap5test

platforms:
  android:
    appName: $APP_NAME
    packageName: $PACKAGE_NAME_ANDROID
    versionName: 2.3.12
    versionCode: 255

But when I deploy the app, it still has the previous name, instead of expected cap5 Test App. Of course this time I tried clean project, rebuild project, and cache invalidation, before I opened this issue.

image

Versions

Trapeze: 7.0.10 Capacitor: 5.0.0

Reproduction

  1. Clone Repo: https://github.com/tobiasmuecksch/cap5-trapeze-bug
  2. Install dependencies npm i
  3. run ionic cap sync
  4. run npx @trapezedev/configure run platform-config.yml
  5. run ionic cap open android
  6. clean project, rebuild the project, invalidate caches and restart Android Studio
  7. Start the app in an emulator
  8. See the aforementioned bug
tobiasmuecksch commented 1 year ago

After some investigation, I found out, that if I alter title_activity_main in android/app/src/main/res/values/strings.xml, the name is applied correctly, which leads me to the conclusion, that trapeze should alter this too.

Additionally we can observe, that package_name remains unchanged in this file too.

image
selcuk-sahin commented 1 year ago

I don't agree that Trapeze should change strings.xml by default, because there is no guarantee those keys will remain there on any project.

Instead, you can edit XML contents inside your trapeze config.yaml file like this:

vars:
  PACKAGE_NAME:
    default: com.myawesomeapp.app
platforms:
  android:
    xml:
      #strings.xml
      - resFile: values/strings.xml
        target: resources/string[@name="app_name"]
        replace: |
          <string name="app_name">$APP_NAME</string>
tobiasmuecksch commented 1 year ago

@selcuk-sahin Well, according to the documentation Trapeze already changes the strings.xml automatically, when in the manifest there is a resource value reference.

image

Source: https://trapeze.dev/docs/Operations/android#appname

Thank you anyways, for this temporary workaround.

selcuk-sahin commented 1 year ago

@selcuk-sahin Well, according to the documentation Trapeze already changes the strings.xml automatically, when in the manifest there is a reference to this file.

image

Source: https://trapeze.dev/docs/Operations/android#appname

Thank you anyways, for this temporary workaround.

Thanks for that, I didn't recognize the documentation has that info.

mlynch commented 1 year ago

All good then? Open to suggestions on tweaking this behavior

tobiasmuecksch commented 1 year ago

@mlynch No. Trapeze does not work as expected. It should update the variables in the strings.xml (as described in the documentation), but currently it does not update them.

That's why appName currently does not have any effect.

mlynch commented 1 year ago

Hmm, tests for this are passing locally. I wonder if there's some difference in the structure of the app that causes it to not work.

mlynch commented 1 year ago

I'm having a hard time following the thread, are you saying that trapeze should also update title_activity_main? You are correct, it's not doing it. But you are seeing it update app_name in strings.xml, correct?

tobiasmuecksch commented 1 year ago

No. In my case strings.xml remains unchanged. That's the problem I've tried to describe.

Feel free to try the provided reproduction repository.

You could also try to remove the capacitor android platform and freshly re add it to your test environment. I assume, that something has changed in the project structure in the past few updates.

tobiasmuecksch commented 1 year ago

@mlynch Have you been able to reproduce the error, or can I help you somehow?

mlynch commented 1 year ago

Just tested it in your app and it works fine for me:

image
tobiasmuecksch commented 1 year ago

@mlynch Yes, but have you started this app in the emulator and inspected it's name in the programs menu? You will see, that app_name does not have the expected effect on the app's name anymore.

I found out that trapeze should also change title_activity_main in strings.xml. I'm sorry, I already mentioned that in https://github.com/ionic-team/trapeze/issues/191#issuecomment-1550257012 but didn't forgot to re-add this information to my prior comment.

tobiasmuecksch commented 1 year ago

@mlynch P.S.: is it expected behavior, that package_name remains unchanged in strings.xml?

mlynch commented 1 year ago

Okay so we need to just have a broader discussion about what appName should do. I have tested your app locally and I see the app_name change taking effect, but the issue here is more about what the OS is displaying and where, and what appName should modify. Right now it's not modifying the main Activity name as well, maybe it should?

image
mlynch commented 1 year ago

@mlynch P.S.: is it expected behavior, that package_name remains unchanged in strings.xml?

I assume you mean when changing the package name? Right now it doesn't look like that string is actually being used anywhere so the tool wouldn't change it. Trapeze is agnostic of the mobile framework so it tries to not assume random strings mean anything in your res files. We can have a separate discussion about package name changes though in a different issue.

tobiasmuecksch commented 1 year ago

@mlynch Thank you for your in-depth responses.

Regarding app_name: Until I updated my app to capacitor 5, the app_name setting also affected the name, which was displayed in the home screen underneath the icon (see first screenshot of the issue). Because this behavior has changed I was confused and thought it would be a bug. But now I do understand why this is not a bug.

In my opinion, developers should at least have a simple option to manipulate the app's displayed name. While keeping in mind what you said about Trapeze's agnosticism, I'd suggest to handle it similar to iOS (where we have a displayName and a productName option). But I'm not quite sure how to name it. How about title_activity?

Regarding package_name: As I'm not very experienced in android development, I only made this observation without understanding whether it has any effect or not. At first sight it seemed odd to me. I'll take some more time to investigate and understand this topic further and create a new issue, in case I still think it's somehow relevant.

gerhardcit commented 1 year ago
xml:
      #strings.xml
      - resFile: values/strings.xml
        target: resources/string[@name="app_name"]
        replace: |
          <string name="app_name">$APP_NAME</string>

At some point this get's worse. when you try and change an app name after Android built it.

npx trapeze run env/app1.yaml 
run android packageName com.amazing.appone
Fatal error: Error running command
Error: Current Java package name and directory structure do not match the <manifest> package attribute. Ensure these match before modifying the project package name
    at AndroidProject.setPackageName (/testing/capacitor2/node_modules/@trapezedev/project/dist/android/project.js:132:19)
    at async executeOperations (/testing/capacitor2/node_modules/@trapezedev/configure/dist/tasks/run.js:52:10)
    at async runCommand (/testing/capacitor2/node_modules/@trapezedev/configure/dist/tasks/run.js:31:9)
    at async /testing/capacitor2/node_modules/@trapezedev/configure/dist/index.js:58:13
    at async Command.<anonymous> (/testing/capacitor2/node_modules/@trapezedev/configure/dist/util/cli.js:31:13)

Surely the idea of this is that you can have one app source (javascript, css etc) and build different app store apps.. ie: Whitelabeling.. I also went with the documentation which does not appear to do what it says it does. Without being able to change the app name.. Trapeze becomes completely useless to us.

gerhardcit commented 1 year ago

I ended up doing something like this in the config-app1.yaml file

vars:
  BUNDLE_ID:
    default: com.amazing.appone
  PACKAGE_NAME:
    default: com.amazing.appone
  APP_NAME:
    default: App One
platforms:
  android:
    packageName: $PACKAGE_NAME
    appName: $APP_NAME
    versionName: 1.1.1
    versionCode: 111
    xml:
      #strings.xml
      - resFile: values/strings.xml
        target: resources/string[@name="app_name"]
        replace: |
          <string name="app_name">$APP_NAME</string>
      - resFile: values/strings.xml
        target: resources/string[@name="title_activity_main"]
        replace: |
          <string name="title_activity_main">$APP_NAME</string>
      - resFile: values/strings.xml
        target: resources/string[@name="package_name"]
        replace: |
          <string name="package_name">$PACKAGE_NAME</string>
      - resFile: values/strings.xml
        target: resources/string[@name="custom_url_scheme"]
        replace: |
          <string name="custom_url_scheme">$PACKAGE_NAME</string>

that produces this output: android/app/src/main/res/values/strings.xml

<?xml version='1.0' encoding='utf-8' ?>
<resources>
    <string name="app_name">App One</string>
    <string name="title_activity_main">App One</string>
    <string name="package_name">com.amazing.appone</string>
    <string name="custom_url_scheme">com.amazing.appone</string>
</resources>

then there is an equivalent for config-app2.yaml npx trapeze run config-app2.yaml -y should change all of appone to apptwo

BUT trapeze does not update the capacitor.config.json or capacitor.config.ts so you would have to manually fix this somehow.

I'm surprised at how capacitor does not understand multi tenant apps from the same source base. Is it that unique?

hcassar93 commented 1 year ago

I just spent hours trying to figure out this exact issue.

Then found this.. silly me 🤦‍♂️

I think @tobiasmuecksch is dead on and his proposal to set title_activity makes sense.

@gerhardcit's solution doesn't work for me as I'm using the project API.

@mlynch thanks for your thoughtful discussion on this thread and generally for your work on ionic and capacitor.

emelampianakis commented 1 year ago

I ended up doing something like this in the config-app1.yaml file

vars:
  BUNDLE_ID:
    default: com.amazing.appone
  PACKAGE_NAME:
    default: com.amazing.appone
  APP_NAME:
    default: App One
platforms:
  android:
    packageName: $PACKAGE_NAME
    appName: $APP_NAME
    versionName: 1.1.1
    versionCode: 111
    xml:
      #strings.xml
      - resFile: values/strings.xml
        target: resources/string[@name="app_name"]
        replace: |
          <string name="app_name">$APP_NAME</string>
      - resFile: values/strings.xml
        target: resources/string[@name="title_activity_main"]
        replace: |
          <string name="title_activity_main">$APP_NAME</string>
      - resFile: values/strings.xml
        target: resources/string[@name="package_name"]
        replace: |
          <string name="package_name">$PACKAGE_NAME</string>
      - resFile: values/strings.xml
        target: resources/string[@name="custom_url_scheme"]
        replace: |
          <string name="custom_url_scheme">$PACKAGE_NAME</string>

that produces this output: android/app/src/main/res/values/strings.xml

<?xml version='1.0' encoding='utf-8' ?>
<resources>
    <string name="app_name">App One</string>
    <string name="title_activity_main">App One</string>
    <string name="package_name">com.amazing.appone</string>
    <string name="custom_url_scheme">com.amazing.appone</string>
</resources>

then there is an equivalent for config-app2.yaml npx trapeze run config-app2.yaml -y should change all of appone to apptwo

BUT trapeze does not update the capacitor.config.json or capacitor.config.ts so you would have to manually fix this somehow.

I'm surprised at how capacitor does not understand multi tenant apps from the same source base. Is it that unique?

I am building capacitor whitelabel apps using capacitor@4 for some time now. The trick is to update the string values after you build & sync the app so the appId and appName in capacitor config do not really matter (correct me if I'm wrong). This is what I'm replacing:

android:
    packageName: $PACKAGE_NAME
    versionName: $VERSION
    versionCode: $ANDROID_VERSION_CODE
    xml:
      - resFile: values/strings.xml
        target: resources/string[@name="app_name"]
        replace: |
          <string name="app_name">$APP_NAME</string>
      - resFile: values/strings.xml
        target: resources/string[@name="title_activity_main"]
        replace: |
          <string name="title_activity_main">$APP_NAME</string>
      - resFile: values/strings.xml
        target: resources/string[@name="package_name"]
        replace: |
          <string name="package_name">$PACKAGE_NAME</string>
      - resFile: values/strings.xml
        target: resources/string[@name="custom_url_scheme"]
        replace: |
          <string name="custom_url_scheme">$PACKAGE_NAME</string>

ios:
    targets:
      App:
        bundleId: $BUNDLE_ID
        version: $VERSION
        buildNumber: $IOS_BUILD_NUMBER
        productName: $APP_NAME
        displayName: $APP_NAME

However with the capacitor@5 changes I encountered the same error as you regarding the manifest. The steps I used to solve it:

the major diff I noticed in the manifest is

-      android:name="com.example.app.MainActivity"
+      android:name=".MainActivity"
lte-deankier commented 1 year ago

Is the strings.xml adjustment the preferred solution? If so, can the documentation be corrected to the appropriate icon and app naming.

tiga05 commented 1 year ago

Hi, I had the same problem with my app, that the strings.xml was not customized and now I stumbled across this discussion here. From my point of view, this should definitely be adapted.

Whatever the decision is: The thread is already half a year old and it should either be pointed out in the documentation or the change should simply be applied.

tobiasmuecksch commented 1 year ago

@tiga05 I'd advise you to create a new issue, since this issue already is too old and too long. I'll close it, as I'm not developing apps anymore.