medic / cht-core

The CHT Core Framework makes it faster to build responsive, offline-first digital health apps that equip health workers to provide better care in their communities. It is a central resource of the Community Health Toolkit.
https://communityhealthtoolkit.org
GNU Affero General Public License v3.0
438 stars 204 forks source link

Fix broken "Reporting Rates" interface in release #1030

Closed ghost closed 8 years ago

ghost commented 9 years ago

From medic/medic-projects#30: the pie-chart "Reporting Rates" interface is obviously broken – we're seeing Javascript errors related to .scrollHeight and .forEach on an undefined object. We should fix this.

garethbowen commented 9 years ago

The UI screenshotted in that issue was fixed in January 2015. I've just fixed a couple of bugs introduced since then and it's mostly working. The major flaw I found is the UI only displays data on the first scheduled form, and provides no way to switch between forms.

garethbowen commented 9 years ago

It's working in develop, but needs a fair bit of migration to get working in a release.

mandric commented 9 years ago

I'll use this issue until it's working in a release.

ghost commented 9 years ago

Update: on Monday, we agreed it'd be a good idea to add facility selection back in, possibly borrowing/reusing the facility selection dropdown from the Reports tab. Mixed in with this issue is some general learning/getting-up-to-speed with Angular.

@mandric: Just for planning purposes, can you give a quick update / remaining effort estimate?

abbyad commented 9 years ago

We already have tentative design discussed already in #1056, just calling out that we should have this feed back into the design process that will yield some mockups for the update.

@mandric, can you check in with @Lesterng about this?

ghost commented 8 years ago

From today's call: we agreed that we'd take the non-routed easy approach to form selection to try to get this shippable by end of day Tuesday (7/3).

mandric commented 8 years ago
( ) confirm anc analytics is unaffected
(x) confirm anc and reporting rates analytics both work together 
( ) pipe all strings through translation
(?) filter health centers based on district selection
(x) remove log statements
(x) confirm dropdowns are not affecting other tabs
(x) show selected values in dropdown 
(x) rename /analytics/stock /analytics/reporting and Stock Monitoring -> Reporting Rates
(?) use Facility instead of FacilityRaw
( ) remove unused kujua-reporting views (maybe kujua-reporting.views.facilities_by_parent)
(x) remove reporting-district-choice related code
( ) remove popup related code in reporting-rates
(?) rename format.facility to format.name
(x) write test and fix tests
(x) fixed direct link to #/analytics/reporting/{{form}}/{{facility}}.
( ) add loading spinner
abbyad commented 8 years ago

@mandric, can you please point me to an instance with data for acceptance testing.

mandric commented 8 years ago

http://52.25.132.140/medic/_design/medic/_rewrite/#/analytics/

abbyad commented 8 years ago

Thanks @mandric, this looks great!

I am digging through for acceptance testing and will post issues here as I go.

On first pass, I see the text in French, regardless of my locale and see this in the console: image

Also, clicking on a form expands to a "Loading..." but get this error in the console: TypeError: exports.info.getForm is not a function

mandric commented 8 years ago

Marc, that locale issue might be be because of the IH Senegal settings I'm using to demo/test are a little weird.

You should never see a "Loading..." maybe you're not looking at a fresh copy? I also just pushed a related fix, please try again.

abbyad commented 8 years ago

As far as the mobile view goes, nice job on making it usable in its current form. There are some UI tweaks for the mobile view but they can likely wait.

Here are some other things I noticed:

(1) The Form filter box should show the form name, not just the form code. For many projects the form codes are not known or don't have special significance, especially by the higher level managers that are looking at the Reporting Rates. This is especially true of projects using Medic Collect or SIM apps, since the form code is only used behind the scenes and never seen by a user. We should reuse the same icon and look as the Reports tab: image

(2) Health Center filter seems confusing to me since it doesn't cascade from the Districts filter. Unless it is a 5 minute task to make this cascading, I would actually remove this filter for now - you can navigate to the Health Center via the district.

(3) Period dropdown would fit well as a top level dropdown, like the "filters". Changing it and having it modify the pie chart above and the content below doesn't feel right.

(4) Blank page when switching between tabs: image screen shot 2015-08-11 at 16 21 34 To reproduce load Reporting Rates, go to another tab, then back to Analytics tab. The page displays properly again after a reloading.

(5) Datetime format should be that which is configured in app_setting image

(6) X Icon looks like a close or a delete: image It may be better to have a warning/exclamation

(7) Reply and Message icons use different icons, but probably better off using the same envelope. We can revisit this during a UI pass at a later time.

abbyad commented 8 years ago

Looping @yembrick in here to provide some feedback for acceptance testing as the Analytics Lead.

ghost commented 8 years ago

Just to be clear, we may not want/need to fix all of the UI/UX issues immediately. Worth a chat during our iteration call (or earlier if needed).

mandric commented 8 years ago

@abbyad thanks for the feedback.

2) I did the health center dropdown to mimic the siblings dropdown that we used to have so you can quickly jump between sibling health centers without going back and forth. It also compensates for the initial index screen going away which lists all forms and facilities.

4) This should be fixed, maybe you were getting cache. Let me know if you can reproduce it after a hard refresh.

abbyad commented 8 years ago

@mandric, do you mind updating the Acceptance Testing Instance to close off acceptance testing?

mandric commented 8 years ago

Should be updated @abbyad.

abbyad commented 8 years ago

Thanks, seeing the updates.

After a successful load, switching Health Centers yields errors and get a blank analytics content section. Here are the console errors: image

abbyad commented 8 years ago

Actually, refreshing the page I get time outs, so it is likely related. Not sure why the instance is not responding anymore. @mandric, are you able to reach http://52.25.132.140/medic/_design/medic/_rewrite/ ?

mandric commented 8 years ago

Now available at https://rc.dev.medicmobile.org/medic/_design/medic/_rewrite#/messages.

mandric commented 8 years ago

After discussion with @abbyad decided to go with removing the health centers list for item (2). A filtered dropdown of health centers (siblings) might be added to the next iteration.

mandric commented 8 years ago

@abbyad I just pushed a new rc have a look when you have a moment.

abbyad commented 8 years ago

Thanks @mandric. After drilling down from the District Level to the Health Center level, how do you go back to the District view? Right now the two ways I can do it is with the browser's Back button, or by reselecting the same district from the District dropdown. This comment is relevant regardless of whether the Health Centers dropdown was removed or not.

It may be useful to have a simple breadcrumbs at the top, or a back button. Unless one of these is actually quick and low risk to implement, I'd say we leave it as for this release with this known issue, and come back to it to do UX/UI updates during a future iteration.

mandric commented 8 years ago

That's true we're missing navigation breadcrumb or two, back button should also work fine though. I'll push forward the release and create another issue for UI tweaks.

mandric commented 8 years ago

We might even hold off on the UI tweaks since we're doing a UI revamp on develop (#1032)?

mandric commented 8 years ago

I'm closing this, we should work on navigation and UI tweaks on the new issues in the newer branches.