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.

Sign up for GitHub

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

Jump to bottom

feat: use compose on login screen #1142

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

feat: use compose on login screen #1142

wants to merge 16 commits into from

Conversation

epicadk
Copy link
Member

@epicadk epicadk commented May 31, 2021 •
edited
Loading

Description

Use jetpack compose for the login screen

Fixes #1139

Type of Change:

  • Code
  • User Interface

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update (software upgrade on readme file)
  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged
  • I have written Kotlin Docs whenever is applicable

Code/Quality Assurance Only

  • My changes generate no new warnings
  • My PR currently breaks something (fix or feature that would cause existing functionality to not work as expected)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been published in downstream modules

@epicadk epicadk requested a review from isabelcosta as a code owner May 31, 2021 18:53
@epicadk epicadk requested a review from annabauza May 31, 2021 18:57
@epicadk
Copy link
Member Author

epicadk commented May 31, 2021

Btw it's compatible with XML Layouts so we can even merge with the base branch.

annabauza
annabauza requested changes Jun 5, 2021
View reviewed changes
Copy link
Contributor

@annabauza annabauza left a comment

Choose a reason for hiding this comment

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

Looks great, I just left some comments. I also wonder if there is no other xmls to remove. Thanks.

app/src/main/java/org/systers/mentorship/view/DefaultSnackBar.kt Show resolved Hide resolved
app/src/main/java/org/systers/mentorship/view/DefaultSnackBar.kt Show resolved Hide resolved
app/src/main/java/org/systers/mentorship/view/activities/LoginActivity.kt Outdated Show resolved Hide resolved
app/src/main/java/org/systers/mentorship/view/activities/LoginActivity.kt Show resolved Hide resolved
app/src/main/java/org/systers/mentorship/view/activities/LoginActivity.kt Outdated Show resolved Hide resolved
@isabelcosta isabelcosta changed the base branch from ui-jetpack-compose to develop June 5, 2021 13:43
@epicadk
Copy link
Member Author

epicadk commented Jun 6, 2021

Sorry a little busy will try to finish this by next week.

vj-codes
vj-codes requested changes Jun 15, 2021
View reviewed changes
Copy link
Member

@vj-codes vj-codes left a comment

Choose a reason for hiding this comment

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

resolve merge conflicts when you're free

@vj-codes vj-codes added the Status: Changes Requested Changes are required to be done by the PR author. label Jun 15, 2021
annabauza
annabauza reviewed Jun 15, 2021
View reviewed changes
app/build.gradle.kts Outdated Show resolved Hide resolved
@epicadk epicadk added Status: Needs Review PR needs an additional review or a maintainer's review. and removed Status: Changes Requested Changes are required to be done by the PR author. labels Sep 19, 2021
@epicadk
Copy link
Member Author

epicadk commented Sep 25, 2021

@isabelcosta @vj-codes please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isabelcosta isabelcosta Awaiting requested review from isabelcosta isabelcosta is a code owner

@annabauza annabauza Awaiting requested review from annabauza

@vj-codes vj-codes Awaiting requested review from vj-codes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
Status: Needs Review PR needs an additional review or a maintainer's review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use jetpack compose in Login
3 participants
@epicadk @annabauza @vj-codes