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

Fix styling on Entry Date row for the table view to display the entry date regardless if it is a same-day charting event #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zplata
Copy link
Contributor

@zplata zplata commented Jun 1, 2017

Currently, in the table view of the Growth Chart app, if there are 2+ different charting events on a patient within the same day, the first charting event will display the date on the "Entry Date" row, while the other charting events on the same day will display a dot instead of the full date (see screenshots below).

While the dot styling helps to show same-day events, it might be more clear to the user to just show the date instead of a dot. This way, someone looking at the time of the entry does not have to guess what exactly the dot represents on the table.

1_before

2_after

… date regardless if it is a same-day charting event
@zplata
Copy link
Contributor Author

zplata commented Jun 1, 2017

If the dot feature was also requested by physicians (similar to #29), we could add a configuration flag for this feature too?

@vlad-ignatov
Copy link
Contributor

Yes, it was requested by them but it was some kind of compromise. The issue was the following:

  1. By design, the first row should display the date and the second - the age. However, in this case they didn't like the date being repeated.
  2. If the time is rendered on the first row instead of date it was too confusing so they preferred to have it on the second row and to have this dot as a way of signifying that there is something special about that measurement...

In my opinion, there is only one reasonable and intuitive solution but it is more complicated. There might be 3 header rows instead of 2 - Date, Time and Age. Multiple records for the same day may have the time displayed on the second row and colspan=n for the date cell. The others can simply have rowspan=2 for the date cells (no time). This is difficult to implement however (considering that the table also supports row and column selections) and we don't plan to do it, unless we have some extra time to spare.

There is also a new version of the app that is currently being developed that might offer different solution.

@zplata
Copy link
Contributor Author

zplata commented Jul 5, 2017

@vlad-ignatov Ah, I see. If you think that this proposal still adds another viable option, we can add a configuration flag to this logic and flex both ways. Otherwise, we can close this PR out if flexing might add more complexity.

@vlad-ignatov
Copy link
Contributor

Yes, I was trying to say that your PR does not exactly fix the UI/UX issues but if someone at Cerner prefers it that way, it won't hurt as long as it is optional.

@zplata
Copy link
Contributor Author

zplata commented Jul 10, 2017

@vlad-ignatov I have added the configuration flag that a user can choose to disable to change the display to what this PR is proposing.

@zplata
Copy link
Contributor Author

zplata commented Nov 6, 2017

@vlad-ignatov Please let me know if there's anything additional needing to be done to contribute this change back. Thank you!

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.

2 participants