Skip to content

AC-734 Fixed the orientation in the Dashboard layout#814

Merged
rishabh-997 merged 1 commit intoopenmrs:masterfrom
sachin-0102:fixLayout
Sep 21, 2020
Merged

AC-734 Fixed the orientation in the Dashboard layout#814
rishabh-997 merged 1 commit intoopenmrs:masterfrom
sachin-0102:fixLayout

Conversation

@sachin-0102
Copy link
Contributor

@sachin-0102 sachin-0102 commented Sep 21, 2020

Description of what I changed

Fixed the orientation of dashboard layout and alignment of text

Issue I worked on

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

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.

Screenshot_2020-09-21-14-42-35-17_0b570628ec88a4202b7916ec2df1bebf.jpg

@sachin-0102 sachin-0102 changed the title Fixed the orientaion in the layout Fixed the orientation in the layout Sep 21, 2020
@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #814      +/-   ##
==========================================
- Coverage   13.64%   13.63%   -0.01%     
==========================================
  Files         237      237              
  Lines        9552     9553       +1     
  Branches      917      918       +1     
==========================================
  Hits         1303     1303              
- Misses       8143     8144       +1     
  Partials      106      106              
Impacted Files Coverage Δ
...chingpatients/MergePatientsRecycleViewAdapter.java 0.00% <0.00%> (ø)
.../org/openmrs/mobile/utilities/PatientComparator.kt 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 13233e8...ecc07da. Read the comment docs.

Copy link
Collaborator

@rishabh-997 rishabh-997 left a comment

Choose a reason for hiding this comment

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

Fix the PR with the changes and they apply to all other card views... Also, claim the task on JIRA first.

@sachin-0102
Copy link
Contributor Author

@rishabh-997 sir please have a look, I've made the required chnages

@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this file required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was auto created on changing the table layout to constraint layout


<androidx.cardview.widget.CardView xmlns:card_view="http://schemas.android.com/apk/res-auto"
android:id="@+id/cardView"
android:id="@+id/cardView1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Explain this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explain this change

When I changed the table layout to constraint layout, the ids changed to cardView5 , cardView6 etc... So I renumbered them from 1 to 5

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the correct file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ids were changed in the entire app when I changed the table layout
So I though it would be best to convert it back in the entire app

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change it back

android:layout_marginLeft="8dp"
android:layout_marginTop="16dp"
android:layout_marginRight="12dp"
android:stretchColumns="1">
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the use of android:stretchColumns here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explain or revert the above queries

On it

android:contentDescription="@string/dashboard_registry_icon_label" />

<TextView
android:id="@+id/FormsLabel"
Copy link
Collaborator

Choose a reason for hiding this comment

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

use camelCase to name view ids.

Copy link
Collaborator

@rishabh-997 rishabh-997 left a comment

Choose a reason for hiding this comment

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

Explain or revert the above queries

@sachin-0102 sachin-0102 force-pushed the fixLayout branch 2 times, most recently from 3cf84a6 to 8ff072d Compare September 21, 2020 15:49


<androidx.cardview.widget.CardView
android:id="@+id/cardView"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@LuGO0, do you think we should name it to something more meaningful?

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 we can make it more descriptive .

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename all 5 card views to functionCardView. Ex findPatientCardView

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay sir I'll do it.

@dino-saurabh
Copy link
Collaborator

dino-saurabh commented Sep 21, 2020

@sachin-0102 The issue is still not claimed,
Follow the Guidelines before starting to contribute read about how to make a PR and the JIRA workflow !!
https://wiki.openmrs.org/display/docs/Pull+Request+Tips

@sachin-0102
Copy link
Contributor Author

@sachin-0102 The issue is still not claimed,
Follow the Guidelines before starting to contribute read about how to make a PR and the JIRA workflow !!

Yes sir. The last comment on the issue said that anyone could take that up so I made a PR, I'll keep that in mind from next time.


<androidx.cardview.widget.CardView xmlns:card_view="http://schemas.android.com/apk/res-auto"
android:id="@+id/cardView"
android:id="@+id/findPatientsCardView"
Copy link
Collaborator

Choose a reason for hiding this comment

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

make it patientCardView

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done sir

Copy link
Collaborator

@rishabh-997 rishabh-997 left a comment

Choose a reason for hiding this comment

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

Read the PR tips carefully... You still lack two changes before I merge it... Ill leave that to you to explore :)
https://wiki.openmrs.org/display/docs/Pull+Request+Tips

@sachin-0102 sachin-0102 deleted the fixLayout branch September 21, 2020 17:54
@sachin-0102 sachin-0102 restored the fixLayout branch September 21, 2020 17:56
@sachin-0102 sachin-0102 deleted the fixLayout branch September 21, 2020 17:57
@sachin-0102 sachin-0102 restored the fixLayout branch September 21, 2020 18:21
@sachin-0102 sachin-0102 reopened this Sep 21, 2020
@sachin-0102 sachin-0102 changed the title Fixed the orientation in the layout AC-734 Fixed the orientation in the layout Sep 21, 2020
Copy link
Collaborator

@rishabh-997 rishabh-997 left a comment

Choose a reason for hiding this comment

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

Nice Work. Follow the instructions carefully next time. Keep contributing, thanks :)

@rishabh-997 rishabh-997 changed the title AC-734 Fixed the orientation in the layout AC-734 Fixed the orientation in the Dashboard layout Sep 21, 2020
@rishabh-997 rishabh-997 merged commit 360d38f into openmrs:master Sep 21, 2020
@sachin-0102
Copy link
Contributor Author

For sure @rishabh-997 sir. Thank you for your guidance.

@sachin-0102 sachin-0102 deleted the fixLayout branch September 22, 2020 03:20
@MasterBlaster99
Copy link
Contributor

Fix the PR with the changes and they apply to all other card views... Also, claim the task on JIRA first.

sir, the claim issue button on jira issue tracker is not there how should i claim on an issue ?

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