-
Notifications
You must be signed in to change notification settings - Fork 6
Add post-18 stage for Ada #1827
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
Conversation
Re-use the Scottish Higher colour to distinguish post-18 from core/advanced
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1827 +/- ##
==========================================
- Coverage 41.73% 41.68% -0.05%
==========================================
Files 542 542
Lines 23685 23688 +3
Branches 7855 7874 +19
==========================================
- Hits 9884 9875 -9
+ Misses 13757 13171 -586
- Partials 44 642 +598 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
[VRT] Update baselines for improvement/post-18-stage
sjd210
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I'll merge the other parts in now and give this a week (with one small request that I might not have asked for ordinarily, but am since we're waiting anyway)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arguably this Advanced context label shouldn't have even shown up in the first place, since "advanced" wasn't a labelled stage for the accordion section (and it's a content problem if "core" or "advanced" is not present, which it is in the vast majority of cases according to my searching).
That said, I think we should be testing this stage-label anyway. Could we either change the mock data so that it does include "advanced", or perhaps switch to "All exam boards" in the context switcher?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, and agreed; I can't see why we should ever default to "advanced", but it would be nice to still test the labels somehow. I'll add "advanced" to the mock data.
(Edit: Changed to your second suggestion ("all exam boards") instead, to avoid needing separate mock contexts for each site)
To allow the accordion stage label to be tested in a VRT
[VRT] Update baselines for improvement/post-18-stage
This shouldn't be merged until the corresponding API change has been in for a release cycle, and potentially not until some content uses the new stage.
The VRT changes are as expected; the mock concept page doesn't actually have an Advanced audience, this is just what we were previously using as a default.