Feature: Add option to attach image from Camera

[shamim-emon]

• Fixes Option to attach from camera #7373

[kewisch]

Quick drive by comment - I believe we shouldn't be adding the strings to the translations, this is taken care of by weblate. I'll let @rafaeltonholo take a closer look at the rest.

[rafaeltonholo]

Hi @shamim-emon,

Thank you for your contribution! I have left a few comments, some of which require further discussion.

Additionally, I kindly ask you to revert all values-{lang}.xml files introduced in this pull request and retain only the values.xml file. Whenever a new string is added to the application, we should first include only the new strings using American English in values.xml. After this is merged, all other language translations should be added through our Weblate project.

For more information, please refer to Translations > Adding a string.

Thanks!

[shamim-emon]

Quick drive by comment - I believe we shouldn't be adding the strings to the translations, this is taken care of by weblate. I'll let @rafaeltonholo take a closer look at the rest.

@kewisch Removed added strings to the translations.

[rafaeltonholo]

Hi @shamim-emon,

Thank you for addressing the other comments! I appreciate your suggestion to move the logic to a dedicated class. However, I have some concerns about creating the module :feature:camera.

According to the Project Structure, a Feature Module should be an independent unit that encapsulates distinct user-facing features. In this context, a camera feature module should offer significant user-facing capabilities, such as camera filters, video clipping, or post-image processing. I don't see these requirements as applicable to our app.

The current code seems to align better with the description of the core module, which mentions "essential utilities and base classes used across the entire project."

Additionally, I would advise against creating a new module for this, as it likely will consist of only a few files.

I suggest placing them in the :core:android:common module, where I believe they would be a better fit.

I would suggest the following structure, inside the :core:android:common module:

• net.thunderbird.core.android.common.camera.CameraCaptureHandler (public api)
• net.thunderbird.core.android.common.camera.provider.CaptureImageFileProvider (internal to the module)
• net.thunderbird.core.android.common.camera.io.CaptureImageFileWriter (internal to the module)

Additionally, you will also need to create a CameraKoinModule.kt, adding the CameraCaptureHandler to the Koin dependency graph. Something like:

// net.thunderbird.core.android.common.camera.CameraKoinModule.kt
internal val cameraModule = module {
    single { CaptureImageFileWriter(context = get()) }
    single<CameraCaptureHandler> {
        DefaultCameraCaptureHandler(
            captureImageFileWriter = get(),
        )
    }
}

// and expose it to the other modules via `app.k9mail.core.android.common.CoreCommonAndroidModule.kt`

val coreCommonAndroidModule: Module = module {
    includes(coreCommonModule)

    includes(contactModule)

    // new Koin module
    includes(cameraModule)
}

Let me know your thoughts on this.

[shamim-emon]

Hi @shamim-emon,

Thank you for addressing the other comments! I appreciate your suggestion to move the logic to a dedicated class. However, I have some concerns about creating the module :feature:camera.

According to the Project Structure, a Feature Module should be an independent unit that encapsulates distinct user-facing features. In this context, a camera feature module should offer significant user-facing capabilities, such as camera filters, video clipping, or post-image processing. I don't see these requirements as applicable to our app.

The current code seems to align better with the description of the core module, which mentions "essential utilities and base classes used across the entire project."

Additionally, I would advise against creating a new module for this, as it likely will consist of only a few files.

I suggest placing them in the :core:android:common module, where I believe they would be a better fit.

I would suggest the following structure, inside the :core:android:common module:

• net.thunderbird.core.android.common.camera.CameraCaptureHandler (public api)
• net.thunderbird.core.android.common.camera.provider.CaptureImageFileProvider (internal to the module)
• net.thunderbird.core.android.common.camera.io.CaptureImageFileWriter (internal to the module)

Additionally, you will also need to create a CameraKoinModule.kt, adding the CameraCaptureHandler to the Koin dependency graph. Something like:

// net.thunderbird.core.android.common.camera.CameraKoinModule.kt
internal val cameraModule = module {
    single { CaptureImageFileWriter(context = get()) }
    single<CameraCaptureHandler> {
        DefaultCameraCaptureHandler(
            captureImageFileWriter = get(),
        )
    }
}

// and expose it to the other modules via `app.k9mail.core.android.common.CoreCommonAndroidModule.kt`

val coreCommonAndroidModule: Module = module {
    includes(coreCommonModule)

    includes(contactModule)

    // new Koin module
    includes(cameraModule)
}

Let me know your thoughts on this.

@rafaeltonholo Done

[rafaeltonholo]

LGTM! Thank you for your contribution to the project!