chore: Generate and display implementation metrics #604
Conversation
jeeyyy
changed the title
|
|
|
I had to refactor quite a bit of pre-processing to be outside of gatsby lifecycle hooks. I moved these to be individual npm scripts. This gives us greater control to keep subsets of functionality and data generation. Added tests & below is the coverage (have plans to improve this stat after this PR).
|
|
Links from rule pages to implementation reports appear to include an anchor reference in the link that isn't jumping to an anchor in the page. Seems to be a bug. eg. https://eloquent-lumiere-13f4f9.netlify.com/rules/97a4e1#implementation-metrics lists 3 reports. The Axe link goes to https://eloquent-lumiere-13f4f9.netlify.com/implementation/axe#97a4e1 which doesn't jump to the relevant part of the report. |
|
@EmmaJP The scroll issue is now resolved. |
| expect(() => getAssertions(null)).toThrow() | ||
| ) | ||
|
|
||
| it('returns assertions from framed reports', async () => { |
There was a problem hiding this comment.
Your tests don't prove that these are assertions, that they are correct, and that you have the right number of them.
| ['@context', '@graph'].forEach((key) => { | ||
| expect(keys.includes(key)).toBe(true) | ||
| }) | ||
| expect(report['@graph'].length).toBeGreaterThan(0) |
There was a problem hiding this comment.
This doesn't prove that you've framed the report correctly.
| expect(result).toBeDefined() | ||
| expect(result.length).toBeGreaterThan(0) | ||
| result.forEach((data) => { | ||
| expect(Object.keys(data)).toEqual(['ruleId', 'ruleName', 'implementation']) |
There was a problem hiding this comment.
How does this tell you if any of these are correct?
| * @param {Object} assertion assertion | ||
| * @param {Object} tc testcase | ||
| */ | ||
| const getTestcaseMapping = (assertion, { expected }) => { |
| * @param {Array<Object>} assertions assertions | ||
| * @param {String} relativeUrl relative url of testcase | ||
| */ | ||
| const getTestcaseAssertions = (assertions, relativeUrl) => { |
There was a problem hiding this comment.
Instead of having this pass the relative URL, why don't you have the test case passed in, have this method figure out the relative URL and decide based on that. Seems like better encapsulation that way.
|
|
||
| &.page-implementers { | ||
| h1, h2, h3, h4, h5, h6 { | ||
| display: inline-block; |
There was a problem hiding this comment.
Headings definitely shouldn't be inline things. Headings are to give a high level structure of the page, not for individual pieces of data.
| </li> | ||
| ) | ||
| })} | ||
| {inputRules.map(inputRuleId => { |
There was a problem hiding this comment.
Please try to keep your logic code a little separated from your UI code. It's hard to read.
| const { context } = sitePage | ||
| const { title: pageTitle, data: contextData } = context | ||
|
|
||
| const updatedTitle = `${pageTitle} | ${site.siteMetadata.title}` |
There was a problem hiding this comment.
Doesn't SEO do this automatically?
5 participants
This PR does the below:
Because of the large nature of the PR. I have deployed this branch to
netlifyto see how implementations are depicted on the site.