Skip to content

Migrate EditSubscriptionActivity to Nav3#662

Merged
ArnyminerZ merged 9 commits into
devfrom
nav3-migration/edit-subscription-activity
Sep 2, 2025
Merged

Migrate EditSubscriptionActivity to Nav3#662
ArnyminerZ merged 9 commits into
devfrom
nav3-migration/edit-subscription-activity

Conversation

@ArnyminerZ
Copy link
Copy Markdown
Member

@ArnyminerZ ArnyminerZ commented Aug 21, 2025

Purpose

Migrates the EditSubscriptionActivity to Nav3.

Short description

  • Added a new destination for editing subscriptions.
  • Got rid of EditSubscriptionActivity.

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

@ArnyminerZ ArnyminerZ self-assigned this Aug 21, 2025
@ArnyminerZ ArnyminerZ requested a review from Copilot August 21, 2025 10:47

This comment was marked as outdated.

Sign in to view

This was referenced Aug 21, 2025
Closed
Closed
@ArnyminerZ ArnyminerZ marked this pull request as ready for review August 21, 2025 10:58
@ArnyminerZ ArnyminerZ requested a review from sunkup August 21, 2025 10:58
sunkup
sunkup requested changes Aug 26, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Code looks good, but for some reason going back from the EditSubscriptionScreen now always closes the app instead of actually going back (checkmark, back arrow and back gesture) to the subscriptions overview.

@ArnyminerZ
Copy link
Copy Markdown
Member Author

ArnyminerZ commented Aug 26, 2025 •
edited
Loading

going back from the EditSubscriptionScreen now always closes the app instead of actually going back

Strange, it is working fine for me 🤔

Screen_recording_20250826_154321.webm

@sunkup
Copy link
Copy Markdown
Member

sunkup commented Aug 27, 2025 •
edited
Loading

Yup, back gesture and back arrow work now, but the checkmark still closes ICSx5 down.

@ArnyminerZ
Copy link
Copy Markdown
Member Author

ArnyminerZ commented Aug 27, 2025

Fixed, for some reason showing the toast message from the composable was being called twice. Now it's called from the viewmodel.

@ArnyminerZ ArnyminerZ requested a review from sunkup August 27, 2025 13:09
sunkup
sunkup requested changes Aug 28, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Nice. Now it works 👍

There is a print statement that should be removed though and then I think it's done.

Comment thread app/src/main/java/at/bitfire/icsdroid/ui/nav/MainApp.kt Outdated
@sunkup sunkup added this to the 2.3.2 milestone Sep 1, 2025
@ArnyminerZ
Merge branch 'dev' into nav3-migration/edit-subscription-activity
f470c1f 
# Conflicts:
#	app/src/main/AndroidManifest.xml
#	app/src/main/java/at/bitfire/icsdroid/model/EditSubscriptionModel.kt
#	app/src/main/java/at/bitfire/icsdroid/ui/nav/MainApp.kt
@ArnyminerZ
Cleanup
f90a408 
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
@ArnyminerZ ArnyminerZ requested a review from Copilot September 1, 2025 10:16
Copilot AI reviewed Sep 1, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the EditSubscriptionActivity to the Nav3 navigation system by replacing the legacy activity-based navigation with composable-based navigation.

  • Removes the EditSubscriptionActivity class and integrates its functionality into the Nav3 navigation system
  • Updates the navigation flow to use composable destinations instead of activity intents
  • Refactors the success message handling from state-based to direct Toast display

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
EditSubscriptionActivity.kt Completely removed the activity class
SubscriptionsScreen.kt Updated to accept navigation callback instead of launching activity
EditSubscriptionScreen.kt Refactored to handle navigation exit and removed success message state
MainApp.kt Added new navigation entry for EditSubscription destination
Destination.kt Added EditSubscription destination data class
EditSubscriptionModel.kt Changed success message handling from state to direct Toast display
AndroidManifest.xml Removed EditSubscriptionActivity registration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread app/src/main/java/at/bitfire/icsdroid/ui/screen/EditSubscriptionScreen.kt
Comment thread app/src/main/java/at/bitfire/icsdroid/model/EditSubscriptionModel.kt
@ArnyminerZ ArnyminerZ requested a review from sunkup September 1, 2025 10:17
sunkup
sunkup requested changes Sep 1, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Looks good!

How about we also update the section comments in the strings file:

  • CalendarListActivity to SubscriptionScreen
  • AddCalendarActivity to AddSubscriptionScreen
  • etc

@ArnyminerZ
Updated comments
c8421b5 
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
@ArnyminerZ ArnyminerZ requested a review from sunkup September 1, 2025 10:45
sunkup
sunkup requested changes Sep 1, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

There is still <!-- settings, currently in CalendarListActivity -->

@ArnyminerZ
Fix missing comment
58f7d08 
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
@ArnyminerZ ArnyminerZ requested a review from sunkup September 2, 2025 09:32
sunkup
sunkup approved these changes Sep 2, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Nice. Mergeable 👍

@ArnyminerZ ArnyminerZ merged commit 655aff7 into dev Sep 2, 2025
7 checks passed
@ArnyminerZ ArnyminerZ deleted the nav3-migration/edit-subscription-activity branch September 2, 2025 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@sunkup sunkup sunkup approved these changes

Assignees

@ArnyminerZ ArnyminerZ

Labels

None yet

Projects

None yet

Milestone

2.3.2

Development

Successfully merging this pull request may close these issues.

3 participants

@ArnyminerZ @sunkup