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

RA-1609:Add condition UI enhancement #209

Merged
merged 2 commits into from Aug 4, 2019

Conversation

@haripriya999
Copy link
Contributor

commented Jul 10, 2019

RA-1609:Add condition UI enhancement

Description of what I changed

Changed UI for add conditions

Issue I worked on

see https://issues.openmrs.org/browse/RA-1609

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).

    No? -> read here on how to squash multiple commits into one

  • My IDE is configured to follow the code style of this project.

    No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend

  • I have added tests to cover my changes. (If you refactored
    existing code that was well tested you do not have to add tests)

    No? -> write tests and add them to this commit git add . && git commit --amend

  • I ran mvn clean package right before creating this pull request and
    added all formatting changes to my commit.

    No? -> execute above command

  • All new and existing tests passed.

    No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.

  • My pull request is based on the latest changes of the master branch.

    No? Unsure? -> execute command git pull --rebase upstream master

@haripriya999 haripriya999 force-pushed the haripriya999:RA-1609 branch from aa7e510 to 0f14a66 Jul 10, 2019
@HerbertYiga

This comment has been minimized.

Copy link

commented Jul 10, 2019

Did this pass locally?

@dkayiwa

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

@haripriya999 of the screenshots on the ticket, which one shows the state of things before your changes?

@haripriya999

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

@dkayiwa I'm sorry I missed to upload the before screenshots. I updated the attachments :)

@dkayiwa

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

Why do you want end date on the right hand side instead of below as it was?

@haripriya999

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

Why do you want end date on the right hand side instead of below as it was?

While toggling the status, it is creating a lot of white space below the onset date when a condition is active, which isn't giving a good look.

@dkayiwa

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

@haripriya999 did that come from the product owner? Or did you discuss it with them?

@haripriya999

This comment has been minimized.

Copy link
Contributor Author

commented Jul 23, 2019

@haripriya999 did that come from the product owner? Or did you discuss it with them?

I'm sorry for delay in reply. I missed your comment on this PR. I didn't understand what you meant? who is the product owner?

@haripriya999 haripriya999 force-pushed the haripriya999:RA-1609 branch from 0f14a66 to 0f44ff8 Jul 27, 2019
@haripriya999

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2019

@dkayiwa Could you please take a look at this. I fixed the merge conflicts. Thank you.

@dkayiwa

This comment has been minimized.

Copy link
Member

commented Aug 4, 2019

Was this a request from @lnball or any one else? Or did you discuss it with them?

@haripriya999

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2019

@dkayiwa I discussed this on talk: https://talk.openmrs.org/t/congrats-for-gsoc/23120/86 , Also @lnball approved these changes.

@dkayiwa dkayiwa merged commit ea26113 into openmrs:master Aug 4, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.