-
Notifications
You must be signed in to change notification settings - Fork 6
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
PN-351: Add comments field so clinicians/group managers can make a note as why the specific match was rejected or saved #211
Conversation
588a65d
to
8679c39
Compare
* @param matchesIds list of ids of matches to mark as notified. | ||
* @return true if successful | ||
*/ | ||
boolean markNotified(Set<Long> matchesIds); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't this method actually needed for the other pull request (#210), which has been merged already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a tiny refactor of the implementation of previous method. Here I moved markNotified and made saveComment REST calls to both go through MatchingNotificationManager. Before I directly called MatchStorageManager for markNotified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll extract this refactor into separate commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed separated commit, changes are moved to PN-349 PR
The status and comment icon need better alignment. Ideally we should have an indicator if there is a comment for each match. Can we show a different icon if there is a comment? Also, both the comment icon and the CONTACT and MARK CONTACTED buttons appear too big and very dominating on the screen. Can we go down with the size to improve the user experience? |
1212625
to
8f959a7
Compare
Added an indicator if there is a comment entered in last commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested
Code reviewed/confirmed. As discussed, some CSS changes can be implemented to make it look better on smaller monitors |
cbd2039
to
d2ff812
Compare
…te as why the specific match was rejected or saved - Added ability to add comment per match via matches table.
…te as why the specific match was rejected or saved Added an indicator if there is a comment entered.
…te as why the specific match was rejected or saved - Grayed out comment buttons. - Update matches and cashed matches on successful comment save.
addressed meeting's codereview comments and rebased on latest master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested. new icons are looking awesome!
Done.