Skip to content

[AC-636] Add a Splash Screen#638

Merged
f4ww4z merged 1 commit intoopenmrs:masterfrom
ribhavsharma:splash-screen
Dec 24, 2019
Merged

[AC-636] Add a Splash Screen#638
f4ww4z merged 1 commit intoopenmrs:masterfrom
ribhavsharma:splash-screen

Conversation

@ribhavsharma
Copy link
Contributor

Description of what I changed

Added a Splash Screen to the app, with an animation.

Issue I worked on

JIRA Issue: https://issues.openmrs.org/browse/AC-636
GCI Task: https://codein.withgoogle.com/dashboard/task-instances/6153655376412672/

Screenshot

ezgif-3-d720da0edfa3

Checklist: I completed these to help reviewers :)

  • My pull request only contains ONE single commit
    (the number above, next to the 'Commits' tab is 1).
  • I have added tests to cover my changes. (If you refactored
    existing code that was well tested you do not have to add tests)
  • All new and existing tests passed.
  • My pull request is based on the latest changes of the master branch.

@codecov-io
Copy link

codecov-io commented Dec 18, 2019

Codecov Report

Merging #638 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #638      +/-   ##
==========================================
- Coverage    14.4%   14.37%   -0.04%     
==========================================
  Files         202      203       +1     
  Lines        9223     9248      +25     
  Branches      791      791              
==========================================
  Hits         1329     1329              
- Misses       7813     7838      +25     
  Partials       81       81
Impacted Files Coverage Δ
.../org/openmrs/mobile/activities/SplashActivity.java 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3989f0...8694540. Read the comment docs.

Copy link
Member

@Akayeshmantha Akayeshmantha left a comment

Choose a reason for hiding this comment

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

LGTM

Please check on the coverage issue as well

import org.openmrs.mobile.R;
import org.openmrs.mobile.activities.login.LoginActivity;


Copy link
Member

Choose a reason for hiding this comment

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

Remove this line, or better to add class level doc here.

Copy link
Member

Choose a reason for hiding this comment

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

Did you address this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @suthagar23. I didn't quite get you. Could you explain it a bit more?

Copy link
Collaborator

@f4ww4z f4ww4z left a comment

Choose a reason for hiding this comment

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

See my comments below @ribhavsharma . Make sure to add the OpenMRS copyright as a comment in new files. You can set up Android Studio to do this for you.

@ribhavsharma ribhavsharma requested a review from f4ww4z December 20, 2019 17:18
import org.openmrs.mobile.R;
import org.openmrs.mobile.activities.login.LoginActivity;


Copy link
Member

Choose a reason for hiding this comment

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

Did you address this comment?

Copy link
Collaborator

@f4ww4z f4ww4z left a comment

Choose a reason for hiding this comment

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

See the comments below @ribhavsharma .

build.gradle Outdated
}
dependencies {
classpath 'com.android.tools.build:gradle:3.5.2'
classpath 'com.android.tools.build:gradle:3.2.0'
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't need to downgrade this.

remove extra lines

fix stuff

remove xtra line + constant

fix
@ribhavsharma ribhavsharma requested a review from f4ww4z December 23, 2019 09:59
Copy link
Collaborator

@f4ww4z f4ww4z 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 to me @ribhavsharma .

@f4ww4z f4ww4z merged commit 4cc4d0d into openmrs:master Dec 24, 2019
rishabh-997 pushed a commit to rishabh-997/openmrs-contrib-android-client that referenced this pull request Dec 27, 2019
rishabh-997 pushed a commit to rishabh-997/openmrs-contrib-android-client that referenced this pull request Dec 28, 2019
AC-560: Wrap Parent layout to match text size in charts list (openmrs#640)

[AC-638] Added inputType for capitalizing the first letter (openmrs#643)

added first time intro slides to app

completed requested changes

added seven intro slides

[AC-638] Added inputType for capitalizing the first letter (openmrs#643)

resolved conflicts

resolved conflicts with splash screen

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants