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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide populationData source strings in app #597

Open
noleti opened this issue Apr 19, 2020 · 14 comments
Open

Provide populationData source strings in app #597

noleti opened this issue Apr 19, 2020 · 14 comments
Labels
help wanted Extra attention is needed t:feat Type: request of a new feature, functionality, enchancement

Comments

@noleti
Copy link
Collaborator

noleti commented Apr 19, 2020

馃檵 Feature Request

  • Provide data source info to users in the app if requestsed

馃敠 Context

  • Allow user to judge quality of data used by us (e.g., if ICU estimated or hard sources)

馃槸 Describe the feature

  • Pull src strings from populationData, inject into scenarios.json, use in app UI (e.g., as part of the help string provided for individual variables)

馃捇 Examples

馃拋 Possible Solution

Related

@noleti noleti added t:feat Type: request of a new feature, functionality, enchancement help wanted Extra attention is needed labels Apr 19, 2020
@realdy
Copy link

realdy commented Apr 19, 2020

Hello, I'm interested in helping out for this feature, but I'm still familiarizing myself with the structure of the repo. Would you mind providing the locations of the files you think would be necessary to add the feature?

@noleti
Copy link
Collaborator Author

noleti commented Apr 19, 2020

The strings are stored in data/populationData.tsv (for many population constraints) and would likely need to be processed in data/scripts/scenarios.py to be included in the generated src/assets/data/scenarios/scenarios.json. From there, they would have to be picked up in the app somewhere. I am not involved in the app development myself, so I can't give more hints afterwards. Maybe we can wait for feedback from @ivan-aksamentov to see if this feature is wanted at the current state before proceeding.

@realdy
Copy link

realdy commented Apr 19, 2020

Okay, thanks for the pointers you've given me so far! It's helpful to have something to work with.

And that makes sense. I could certainly see this feature as being helpful, but there might be a lack of visibility in its current proposed form. It's not immediately obvious to the user that clicking on the ICU/ICMU question mark button would show the user where the data is sourced from. So getting some feedback before making a solid build would be ideal.

That said, I'll still go ahead attempting to make progress since it would be helpful to understand the structure of the repo and I think that including the information somewhere would still be helpful, even if it's not in the originally proposed form.

@realdy
Copy link

realdy commented Apr 20, 2020

I managed to create a commit that includes the srcHospitalBeds parameter in the generated json file if you would like to review it.

Working on adding the relevant data to the app

@realdy
Copy link

realdy commented Apr 20, 2020

To get some verification, when you were thinking of the structure of the feature, did you want it to look something like this:
Example

The idea is that whenever a location is selected, the text in the help box will refresh based off the data in the json file

@noleti
Copy link
Collaborator Author

noleti commented Apr 20, 2020

Yes, although the existing 'Presets are rough estimates...' should ideally be replaced by the source string.

@realdy
Copy link

realdy commented Apr 20, 2020

Okay, I'll try to give it my best shot at incorporating the changes into the UI.
I've currently identified ScenarioCardPopulation.tsx and scenarioData.ts as being the likely files I'll need to edit to apply the necessary changes, but there's probably some other files I'll have to interact with in order to have the description boxes detect when the location changes and update the description.

@nnoll
Copy link
Collaborator

nnoll commented Apr 21, 2020

I like this idea. One important point is the help button must revert to it's previous state as soon as the user enters a custom value. I'm not sure how easy this would be to manage as now every button would need to manage state. I would think a citation button (much like Google scholar) at the header of each card would work and would be easier

@ivan-aksamentov
Copy link
Member

I propose to just dump all the sources and citations on a dedicated page #108. I don't think we need to add the unnecessary fluff to the simulator app itself.

It will interact with the current scenario and case count state, as well as with the cases import feature #381 and params import through files #541, url or otherwise. There may be corner cases: custom names and missing citations. We need to figure out the imports UX first.

@realdy
Copy link

realdy commented Apr 21, 2020

I agree with that to an extent. Perhaps putting the src data in their own visible location would be easier to work with, since it would be more visible and obvious to the user where the data is coming from rather than clicking on the question button.

Though I think user experience would be improved by having the specific source citation pop up upon the click of a button rather than just having an entire page the user has to look through. Dumping all the data would be kind of no different from having the user look through the populationData.tsv, though I suppose it has the benefit of compiling all the data in one location rather than them having to click between locations to find individual sources or actually locating the populationData.tsv.

So on that note, why not implement both? The citation page could be entirely its own separate thing for users looking for the compiled sources, but the popup button could be there as well if users want to quickly locate the data.

@realdy
Copy link

realdy commented Apr 21, 2020

So I've gotten the UI mostly working, but there's a small bug that occurs with typescript and strings when a colon appears in the string. The full content of the below string should be "Computed from 2014 data in CIA fact book https://web.archive.org/web/20191122111714/https://www.cia.gov/library/publications/the-world-factbook/geos/af.html"

But because of the placement of the colon, it messes up the message for whatever reason - as shown below.
Example

Not sure how to fix it so does anyone have any suggestions? I originally was going to typecast the string into a template string and use String.raw, but that didn't seem to do the trick.

Also, does anyone know how to get newline characters to appear in the help box? Newline characters don't seem to show up either, and I'm hoping there's some simple solution that wouldn't require having me to look through css and other files.

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Apr 21, 2020

@realdy I cannot tell without seeing the code. If you haven't yet, make a draft pull request so that it can be reviewed. Typecasts and String.raw will not be accepted. Make sure you lint your code with yarn eslint as well.

I could not say for certain if this feature will be accepted or not, so proceed at your own risk :)

@realdy
Copy link

realdy commented Apr 21, 2020

Okay so after many commits and bad decisions that I subsequently fixed, got a functioning build that conforms to the yarn eslint standards. #606 The code still has the errors I noted earlier and I made sure to comment within ScenarioCardPopulation the relevant area to fix.

Aside from bugs related to the ':' character and newlines not being included properly, the help boxes do display properly according to the scenario and I made sure that if the current scenario is in custom, nothing appears for the sources

@realdy
Copy link

realdy commented Apr 22, 2020

Just fixed the colon rendering issue, but still trying to figure out how to add newline characters to make things more visually appealing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed t:feat Type: request of a new feature, functionality, enchancement
Projects
None yet
Development

No branches or pull requests

4 participants