Final text bugs fix. - Githubissues

dfe-analytical-services / leo-graduate-industry-dashboard

Source code for the Longitudinal Education Outcomes (LEO) graduate industry dashboard.

https://department-for-education.shinyapps.io/leo-graduate-industry-dashboard/

3 stars 2 forks source link

Final text bugs fix. #78

Closed chfoster closed 2 years ago

chfoster commented 2 years ago

Pull request overview

Complete overhaul of regional text code to take into account NAs, 0s and multiple regions tying first/last.
Add validate statement to regional Sankey to avoid error (instead show 'there is no data' text').
Add footnote to sankey about rounding to nearest 5 FTE.
Fix the error currently seen in industry flow text.

Pull request checklist

Please check if your PR fulfils the following:

[x] Tests have been run locally and are passing (run_tests_locally())
[x] Code is styled according to tidyverse styling (checked locally with tidy_code())

What is the current behaviour?

What is the new behaviour?

Anything else

@hazmut, please could you review the new fixes and I will work on addressing those issues already raised on subject by industry text.

hazmut commented 2 years ago

Just checking this example of 0's - regions with studied in region=0 are listed in text as the regions with fewest graduates, but are they supposed to be? (suppose it's not technically wrong, it's just that fewest = none at all!)

Perhaps the above example should be like this one, where the studied in/living in sentences for regions with fewest graduates aren't included?

hazmut commented 2 years ago

When there is no data (as in the 'new behaviour' example screenshot above), there is just a full stop for the text. Is this right, or should there be nothing at all, or perhaps 'There is no data for this selection'?

hazmut commented 2 years ago

Is it possible to format the legend box on the map so the grey NA box isn't so close to the bar on the left? (Just a thought - not a big deal if it's not easily done)

chfoster commented 2 years ago

Just checking this example of 0's - regions with studied in region=0 are listed in text as the regions with fewest graduates, but are they supposed to be? (suppose it's not technically wrong, it's just that fewest = none at all!)

Perhaps the above example should be like this one, where the studied in/living in sentences for regions with fewest graduates aren't included?

Going to leave this as I think it's okay to say those that have 0 have the least graduates (there may be some cases in which there is just one region with 0 - we would want to identify that as having the least!)

chfoster commented 2 years ago

When there is no data (as in the 'new behaviour' example screenshot above), there is just a full stop for the text. Is this right, or should there be nothing at all, or perhaps 'There is no data for this selection'?

Have ammended to say 'there is no summary for this selection' as in other tabs.

chfoster commented 2 years ago

Is it possible to format the legend box on the map so the grey NA box isn't so close to the bar on the left? (Just a thought - not a big deal if it's not easily done)

Going to leave this since it's purely aesthetic

chfoster commented 2 years ago

I think it just needs a small tweak to say when both studied in/lived in are NA then the difference is still NA.

Tweaked as suggested! @hazmut

hazmut commented 2 years ago

The colours have gone a bit odd in this example, as grey is included in the colour scale rather than being only for NA. Not sure if it was maybe fixing 0 to white on the colour scale caused this?!

chfoster commented 2 years ago

The colours have gone a bit odd in this example, as grey is included in the colour scale rather than being only for NA. Not sure if it was maybe fixing 0 to white on the colour scale caused this?!

Fixed colour scale to not include any grey!