Naive solution to embedding iframes into example pages#12
Naive solution to embedding iframes into example pages#12
Conversation
alflennik
left a comment
There was a problem hiding this comment.
As you can see in the screenshot, I got the report to appear!
It's a POC so we're basically just proving that it can work, so I don't have a ton of feedback, because it clearly works. One thing I do think we should fix is just the fact that the axios dependency is not in the package.json. I think it worked for you because axios is installed globally? I had to add it for it to work for me.
Okay, and then the assumptions you've listed out.
- It's true that we're assuming that the data comes from production. This doesn't bother me actually. I think it would be conceptually simpler to completely ignore the ARIA-AT App staging environments and pretend they don't exist.
- The assumption about git shas seems right. I do think there is a slightly cleaner way to write the graphql query for when we implement this for real, but also there is an opportunity to clean up the query even more just by adding a couple fields to the API.
- Instead of using the TestPlan id, perhaps in the future we can use the exampleURL which is in the metadata object.
- I don't think this is inefficient! The main thing that might cause bad performance would be if we were making network requests per example, and fortunately you sidestepped that and did one global request for all examples. Nice.
howard-e
left a comment
There was a problem hiding this comment.
Looks good!
In addition to @alflennik comments and given that this is a POC - for the future, I think it would be worth exploring aria-at-app providing a basic version of the Results + Complete Results tables made specifically for an embed such as this.
|
|
||
| const { title, head, body, outline, relatedLinks } = getContent(); | ||
|
|
||
| const testReport = reportId ? `<div> |
There was a problem hiding this comment.
Seems like this should be a <section>
| <div class="sidebar-right">${body}</div> | ||
| <div class="sidebar-right"> | ||
| ${body} | ||
| ${testReport} |
There was a problem hiding this comment.
This conditionally appends the Test Report section to the body after the navigation link back to the design pattern. It seems to me that we'd want to preserve that always showing up at the end in the future
| `; | ||
| }) | ||
| .join(" ")} | ||
| <li><a href="#test-report">Test Report</a></li> |
There was a problem hiding this comment.
This should be conditionally rendered as well or the Test Report menu link will show in cases where the Test Report section hasn't been rendered
fc0acac to
58f182c
Compare
58f182c to
e12eba0
Compare
alflennik
left a comment
There was a problem hiding this comment.
Thanks for addressing my note Erika! I think the plan, if I understand it right, is that we'll keep this unmerged, since it's a POC, but each of us will approve it once we verify you've addressed our comments.

Proof of Concept
This PR is part of this Asana task. It is a naive solution to embedding iframes from ARIA-AT app to the WAI ARIA Practices site. Some assumptions and shortcuts taken for this proof of concept are:
Open questions