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

AC-763 Updated UI for splash screen #744

Merged
merged 3 commits into from
Jun 2, 2020

Conversation

rishabh-997
Copy link
Collaborator

Description of what I changed

  1. Converted to constraint layout
  2. Used View Binding
  3. New UI with new font

Issue I worked on

JIRA Issue: https://issues.openmrs.org/browse/AC-763

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.

@rishabh-997
Copy link
Collaborator Author

WhatsApp Image 2020-05-17 at 9 35 48 PM

@codecov-commenter
Copy link

codecov-commenter commented May 19, 2020

Codecov Report

Merging #744 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #744      +/-   ##
==========================================
- Coverage   11.80%   11.80%   -0.01%     
==========================================
  Files         238      238              
  Lines       10647    10650       +3     
  Branches     1028     1028              
==========================================
  Hits         1257     1257              
- Misses       9308     9311       +3     
  Partials       82       82              
Impacted Files Coverage Δ
...mobile/activities/introduction/SplashActivity.java 0.00% <0.00%> (ø)

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 d9c3f5c...c86070a. Read the comment docs.

@HerbertYiga
Copy link

@rishabh-997 thanks for this,you can also add a ticket ID on your commit message

openmrs-client/src/main/res/layout/activity_splash.xml Outdated Show resolved Hide resolved
binding.organizationName.setTypeface(typeface);
binding.organizationName.setText(R.string.organization_name);

binding.clientName.setTypeface(typeface);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@rishabh-997 why not assigning the text and fonts directly in the xml file, it will reduce the code here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This font is Monserrat which is not available in default font family so had to add the .tff file to use it...

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rishabh-997 Move line 46 to FontsUtil and just retrieve the Typeface here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But we discussed on removing the entire FontUtils.java file ???

Copy link
Collaborator

Choose a reason for hiding this comment

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

But we discussed on removing the entire FontUtils.java file ???

Ok this is fine for now, but if we want to add more special fonts we might have to recreate FontUtils.

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.

@rishabh-997 All good except 1 thing.

binding.organizationName.setTypeface(typeface);
binding.organizationName.setText(R.string.organization_name);

binding.clientName.setTypeface(typeface);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@rishabh-997 Move line 46 to FontsUtil and just retrieve the Typeface here.

@f4ww4z
Copy link
Collaborator

f4ww4z commented Jun 1, 2020

@rishabh-997 LGTM except the merge conflict.

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.

binding.organizationName.setTypeface(typeface);
binding.organizationName.setText(R.string.organization_name);

binding.clientName.setTypeface(typeface);
Copy link
Collaborator

Choose a reason for hiding this comment

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

But we discussed on removing the entire FontUtils.java file ???

Ok this is fine for now, but if we want to add more special fonts we might have to recreate FontUtils.

@f4ww4z f4ww4z merged commit fff0a18 into openmrs:master Jun 2, 2020
@rishabh-997 rishabh-997 deleted the AC-763-splashUI branch June 6, 2020 04:04
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