Update docs #627
Update docs #627
Conversation
Deploying airlock-docs with
|
| Latest commit: |
411d634
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5f7cfb83.airlock.pages.dev |
| Branch Preview URL: | https://docs.airlock.pages.dev |
This will likely be updated later to be included in a more explanatory doc page.
Reorganise existing docs into the how-tos, explanation and reference subfolders. Create empty placeholder docs where necessary. No significant changes to the actual content itself yet.
Add a helper function for screenshotting an element with some padding, and screenshot the login form during a test. My plan is to generate screenshots during the functional tests to use for the docs. This should keep any screenshots in the docs up to date with UI changes and styling. It might require some future changes to the test setup to make the screenshots have a bit more realistic data in them.
These pages are also moved to the reference section, as they are more of an extra reference than a how-to guide for a specific task. This also means we can refer to both workspace and request files and avoid some duplication.
To differentiate them from the screenshots that are auto-generated during the functional tests.
Consolidates the update and withdraw file pages into one "edit file" how-to, which also includes how to change a file type or group. These two are identical, but are in their own sections in anticipation of moving a file between groups becoming a thing that we allow users to do in future.
Screenshots were scattered around the functional tests, which made it difficult to identify where each screenshot used in the docs came from. They are now all in dedicated tests, which avoid unnecessary clicking around and asserting content as much as possible. This does mean that the screenshot tests duplicate quite a lot of the e2e tests. However, we don't necessarily want to run them by default, because many of them will change on every run (the request ID will change, and timestamps of files and activity logs also). There are some hacky things I experimented with to fix the request ID and the timestamps, but I think it's probably better that we only run these tests to update screenshots when we know we want to do so. The test can run during CI to ensure that there are no actual errors in it that need to be fixed up. Not fixing the timestamps has the additional benefit that the timestamps can serve as a record of when the screenshots were last updated.
They can be run by setting an enviroment variable. Run the screenshot tests in CI; they will generate new screenshots, but we only care (in CI) about checking that they don't fail.
Workflow and permissions, notifications, why-airlock page
There was a problem hiding this comment.
This is great work. A really solid foundation, both in terms of doc structure, and the playwright tooling to re-screenshot.
I think we may want to add some extra bits or finesse some wording in a few places, the structure and flow is very good. For review, I main looked at the built pages. I've left a few comments, but they are for later PR.
|
|
||
| --- | ||
|
|
||
| * Next: [View workspace files](view-workspace-files.md) |
There was a problem hiding this comment.
nit: because workspace is in the glossary, in the text of this link, and others like it, there are some odd artifacts:
- mixed underlining between link and acronym styles
- acronym pointer/link pointer fighting
Not sure what the best solution is here, and its a small thing. If we gave link styling greater priority, perhaps that would work?
There was a problem hiding this comment.
I noticed this too, and I wasn't sure what the best solution was. It happens in the main docs too in places, I think.
|
|
||
|  | ||
|
|
||
| In the dialogue that opens, specify the file group you wish to add the file(s) to, and |
There was a problem hiding this comment.
Would this be a good time to explain what a file group is, and what kind of name you should give one?
There was a problem hiding this comment.
I had the same thought, but following the diataxis guidelines, how-tos should tell you how to do a thing, not necessarily explaining why. This should maybe be a page in the Explanation section about file groups that we could link to?
|
|
||
| At this stage, more comments can be added or the visibility of existing | ||
| comments can be changed. Output checkers should determine the set of | ||
| comments that they intend to return to the researcher (if any). |
There was a problem hiding this comment.
Should we have a section here saying the the reviews need to discuss and agree on the decisions for any files where they disagreed during independent review?
There was a problem hiding this comment.
Same here - this could link to an explanation doc about independent review. Although that does perhaps stray a bit too far into output checker process, and we don't really want to maintain output checking docs within airlock. Should this link back to the main docs? (I'd been trying not to do that as much as possible, because the main docs aren't accessible from L4 airlock docs)
Add add the ability to crop the height/width to a percentage.
There was a problem hiding this comment.
I've not reviewed in depth but I've read through all the pages and I think that this goes into an appropriate level of detail.
One suggestion: I find the use of "explanation" a bit jarring. Could we change it to "In depth"?
Thanks for all the work you've put into this.
Updating the airlock docs:
Goes along with opensafely/documentation#1594