Skip to content

Conversation

@snaik20
Copy link
Contributor

@snaik20 snaik20 commented Jun 26, 2025

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

This PR refines the Settings screen setup using Jetpack Compose:

  • Navigation Routing: Updated navigation logic to use Serialized Routes, as recommended in the official Compose navigation documentation.
  • Screen Title Management: Migrated screen title handling to a Compose-native approach instead of using addOnDestinationChangedListener.
  • Toolbar Enhancements:
    • Refined the toolbar layout and styling to improve alignment with Material theming.
    • Implemented proper ripple effects and consistent color usage.
    • Fixed back navigation behavior to align with Compose conventions.
  • Theming Note: The current Compose-based theme system does not yet fully replicate the color palette used in the legacy (pre-Compose) implementation, especially across different services. A TODO has been added in code, and a separate task will be created to address this comprehensively.

Before/After Screenshots/Screen Record

  • Before:
  • After:

Fixes the following issue(s)

  • Fixes #

Relies on the following changes

  • N/A

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.

Due diligence

@snaik20 snaik20 requested a review from Stypox June 26, 2025 05:38
@github-actions github-actions bot added the size/medium PRs with less than 250 changed lines label Jun 26, 2025
@ShareASmile ShareASmile added codequality Improvements to the codebase to improve the code quality rewrite Issues and PRs related to rewrite labels Jun 26, 2025
@github-project-automation github-project-automation bot moved this to In Progress in Rewrite Jun 26, 2025
@snaik20 snaik20 force-pushed the refine_settings_compose_setup branch 4 times, most recently from f6f594e to d4751ee Compare July 9, 2025 20:33
@ShareASmile ShareASmile added the ready for review Most of the work is done, PR is now ready for a review label Jul 12, 2025
@snaik20 snaik20 force-pushed the refine_settings_compose_setup branch from d4751ee to 47ddaed Compare August 22, 2025 19:07
@snaik20 snaik20 force-pushed the refine_settings_compose_setup branch from 47ddaed to 39a89d4 Compare September 24, 2025 18:36
Comment on lines 85 to 88
colors = TopAppBarDefaults.topAppBarColors(
containerColor = MaterialTheme.colorScheme.primary,
titleContentColor = MaterialTheme.colorScheme.onPrimary,
),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better remove this, since I have raised a PR to migrate to the Material 3 color scheme: #12654

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Will update this PR after your Material 3 PR is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undone the toolbar colors to unblock this PR as the Theming is being handled in #12654. Let us handle the Jetpack compose theming as well in your PR.

@theimpulson theimpulson self-assigned this Oct 11, 2025
@theimpulson theimpulson self-requested a review October 11, 2025 04:06
@theimpulson
Copy link
Member

It would be nice to switch to navigation3 to avoid more migration work whenever that hits stable. I will do that afterwards.

@snaik20
Copy link
Contributor Author

snaik20 commented Oct 22, 2025

It would be nice to switch to navigation3 to avoid more migration work whenever that hits stable. I will do that afterwards.

That would be nice. I can take a look at Navigation 3. Quick question: are we looking for a specific feature in Navigation 3, or are we considering it mainly because it is newer and designed for Jetpack Compose? I’m a bit skeptical as the official documentation mentions that the library is still in alpha.

This PR refines the Settings screen setup using Jetpack Compose:

* **Navigation Routing**: Updated navigation logic to use **Serialized Routes**, as recommended in the [official Compose navigation documentation](https://developer.android.com/develop/ui/compose/navigation).
* **Screen Title Management**: Migrated screen title handling to a Compose-native approach instead of using `addOnDestinationChangedListener`.
* **Toolbar Enhancements**:
  * Refined the toolbar layout and styling to improve alignment with Material theming.
  * Implemented proper ripple effects and consistent color usage.
  * Fixed back navigation behavior to align with Compose conventions.
* **Theming Note**: The current Compose-based theme system does not yet fully replicate the color palette used in the legacy (pre-Compose) implementation, especially across different services. A `TODO` has been added in code, and a separate task will be created to address this comprehensively.
@snaik20 snaik20 force-pushed the refine_settings_compose_setup branch from 39a89d4 to d06244f Compare October 22, 2025 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codequality Improvements to the codebase to improve the code quality ready for review Most of the work is done, PR is now ready for a review rewrite Issues and PRs related to rewrite size/medium PRs with less than 250 changed lines

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants