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

Rewrite PermissionsIntroPage to M3 #758

Conversation

ArnyminerZ
Copy link
Member

@ArnyminerZ ArnyminerZ commented Apr 28, 2024

(See #691)

Purpose

Update design of the PermissionsIntroPage intro page (and PermissionsActivity) so that it uses M3.

Note

BasicTopAppBar still has to be migrated to M3. May be good to migrate it together with this PR since it basically has the same considerations as CardWithImage.

Short description

  • Replaced all M2 usages with M3 of PermissionsCardContent
  • Replaced all M2 usages with M3 of CardWithImage.
    More intro pages will benefit from this change. May have effects on Rewrite DebugInfoActivity to M3 #744, however, I haven't seen any modifications here, and this is a smaller PR, so maybe we should merge this here.
  • Forced all states in PermissionsActivity.Model to have private setters.

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 added the refactoring Internal improvement of existing functions label Apr 28, 2024
@ArnyminerZ ArnyminerZ self-assigned this Apr 28, 2024
@ArnyminerZ ArnyminerZ linked an issue Apr 28, 2024 that may be closed by this pull request
@ArnyminerZ ArnyminerZ requested a review from rfc2822 April 28, 2024 09:10
@ArnyminerZ ArnyminerZ marked this pull request as ready for review April 28, 2024 09:10
@ArnyminerZ ArnyminerZ mentioned this pull request Apr 28, 2024
4 tasks
@rfc2822 rfc2822 requested review from sunkup and removed request for rfc2822 April 28, 2024 13:56
@sunkup
Copy link
Member

sunkup commented Apr 29, 2024

Will review the PR after its dependency has been merged

@rfc2822 rfc2822 force-pushed the main-ose branch 17 times, most recently from 47ef18b to 39f8f2e Compare May 2, 2024 12:03
ArnyminerZ added 2 commits May 4, 2024 13:04
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
ArnyminerZ and others added 4 commits May 4, 2024 13:04
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
@rfc2822 rfc2822 force-pushed the 757-rewrite-permissionsintropage-to-m3-→-permissionsactivity branch from 91a56f9 to 7836bd7 Compare May 4, 2024 11:04
@rfc2822 rfc2822 assigned rfc2822 and unassigned ArnyminerZ May 4, 2024
Copy link
Member

@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

I did a few final changes, most notably:

  • Observe the lifecycle directly from the Composable, so that onResume works regardless of the Activity where the Screen was embedded from. It also means that PermissionsActivity doesn't need to create/pass the model.
  • M3 buttons are not uppercase() anymore, please always change that

@github-actions github-actions bot removed the dependent label May 4, 2024
@rfc2822 rfc2822 merged commit 4cffbe7 into main-ose May 4, 2024
8 checks passed
@rfc2822 rfc2822 deleted the 757-rewrite-permissionsintropage-to-m3-→-permissionsactivity branch May 4, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Internal improvement of existing functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite PermissionsIntroPage to M3 → PermissionsActivity
3 participants