Special 'no estimates' message for meninigitis vaccines#74
Special 'no estimates' message for meninigitis vaccines#74david-mears-2 merged 12 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #74 +/- ##
==========================================
+ Coverage 98.65% 98.68% +0.03%
==========================================
Files 26 26
Lines 595 610 +15
Branches 146 152 +6
==========================================
+ Hits 587 602 +15
Misses 7 7
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…imc/vaxviz into mrc-6891-helpful-no-estimates-messages
EmmaLRussell
left a comment
EmmaLRussell
left a comment
There was a problem hiding this comment.
Did you also want to include a meningitis special message in the case where some focuses do show results, but one or more meningitis selection does not (in the blue alert beside the legend)?
Also - I've just spotted that if you have 2 focus selections, one of which has no data, you don't see a legend. The user can work out from context of the alert message which one isn't missing, but it might be clearer to show the with-data selection on the legend.
| </p> | ||
| <FwbAlert v-else-if="noDataToDisplay" icon class="w-fit m-auto mt-10"> | ||
| <span v-if="appStore.focuses.length && appStore.focuses.every(f => meningitisVaccines.includes(f)) && !appStore.splitByActivityType"> | ||
| Estimates for {{ appStore.focuses.join(", ") }} are only available at the campaign/routine level.<br/> |
There was a problem hiding this comment.
| Estimates for {{ appStore.focuses.join(", ") }} are only available at the campaign/routine level.<br/> | |
| Estimates for {{ appStore.focuses.join(", ") }} are only available at the activity type level.<br/> |
Don't have strong feeling, but this change would be a little more consistent. But as it is, it gives more context.
There was a problem hiding this comment.
changing to:
Estimates for {{ appStore.focuses.join(", ") }} are only available at the activity type (campaign/routine) level.
| <FwbAlert v-else-if="noDataToDisplay" icon class="w-fit m-auto mt-10"> | ||
| <span v-if="appStore.focuses.length && appStore.focuses.every(f => meningitisVaccines.includes(f)) && !appStore.splitByActivityType"> | ||
| Estimates for {{ appStore.focuses.join(", ") }} are only available at the campaign/routine level.<br/> | ||
| Try selecting ‘Split by activity type’, or view meningitis estimates (a composite of MenA and MenACWYX). |
There was a problem hiding this comment.
Shouldn't we upper case here, as it is in the legend and select control?
| Try selecting ‘Split by activity type’, or view meningitis estimates (a composite of MenA and MenACWYX). | |
| Try selecting ‘Split by activity type’, or view Meningitis estimates (a composite of MenA and MenACWYX). |
There was a problem hiding this comment.
My thinking is that it should be upper case in the legend and select control because it's the first word in a 'sentence' or 'title'; but here, it's not the first word in a sentence or title.
I do think my brain has got polluted to always spelling diseases with capital letters, from seeing them in sentence case so often, but that's not actually normal, see e.g. https://www.nhs.uk/conditions/meningitis/
There was a problem hiding this comment.
Fair point - but an app isn't prose, and I don't think "Meningitis" here is functioning just like a word in a sentence. I feel like the eye gets used to the shape of identifiers like "Meningitis" in the app, appearing in different contexts, and it's then jarring to see that same identifier modified by case. I think that's particularly true here because you're instructing the user to "view meningitis estimates", but they need to select "Meningitis" to do that (which also highlights that "Meningitis" refers to the set of estimates not the disease meningitis...)
It also seems weird because of the vaccines retaining title case...
I may be overthinking this though! 🤯
There was a problem hiding this comment.
I've noticed the previous sentence is running a chance of including a capitalised disease, since it interpolates appStore.focuses.join(", "). So I will spell 'Meningitis' to match that.
But in any case... I think lowercase feels jarring to us because our exposure to the word 'meningitis' is generally in such apps as this, where it's ~always a label instead of in a sentence. For scientists and users, their exposure to the word will be more general.
And I do think the vaccines are spelt with capitals for whatever reason - this is the style I see at https://www.meningitis.org/about-meningitis/vaccines/meningococcal-vaccines/
|
Added a message for the no-data case when meningitis is the focus and there is a split by activity type. |
Added alerts for these cases. Made the alerts that are adjacent to the legend closable otherwise they can pile up. Edited: These alerts are now concatenated to a single potentially verbose alert. This is no longer closable. |
This is to make the legend still show if there are >1 focuses but only 1 of them has data.
Updated the color legend showing logic. |
david-mears-2
requested review from
EmmaLRussell
and removed request for
EmmaLRussell
EmmaLRussell
left a comment
EmmaLRussell
left a comment
There was a problem hiding this comment.
All looks good, just the issue of deleting all items from legend for another ticket.
2 participants
As discussed with Katy, it would be good to make the 'no estimates' message more helpful in the case of MenA and MenACWYX, which are both meningitis vaccines.
I welcome feedback on the phrasing: "Estimates for {{ MenA / MenACWYX }} are only available at the campaign/routine level. Try selecting ‘Split by activity type’, or view meningitis estimates (a composite of MenA and MenACWYX)."