#7493 Document Builder custom styles Slice 3: Attach stylesheets to the rendered document #7507
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7507 +/- ##
==========================================
+ Coverage 72.64% 72.66% +0.01%
==========================================
Files 1212 1212
Lines 37956 37961 +5
Branches 7126 7128 +2
==========================================
+ Hits 27575 27584 +9
+ Misses 10381 10377 -4 ☔ View full report in Codecov by Sentry. |
| @@ -102,6 +103,7 @@ export class DocumentRenderer extends RendererABC { | |||
| Component: DocumentViewLazy, | |||
| props: { | |||
| body, | |||
| stylesheets: compact(stylesheets).filter((url) => isValidUrl(url)), | |||
There was a problem hiding this comment.
I think we want to throw BusinessError on any invalid URLs vs. silently ignoring?
| "/DocumentView.css", | ||
| bootstrap, | ||
| bootstrapOverrides, | ||
| ...stylesheets, |
There was a problem hiding this comment.
Open for the definition of done debate, but my original intent was to exclude the base bootstrap stylesheet if any stylesheet overrides are passed in.
There was a problem hiding this comment.
Ok, I didn't see that in the spec, but happy to make that change here. 👍
There was a problem hiding this comment.
Actually, there are some implications of this that would change the scope of this quite a bit I think. I'll make a new slice to address this at the end.
There was a problem hiding this comment.
Approved to unblock dependent slices. See specification clarification here: #7507 (comment)
|
When the PR is merged, the first loom link found on this PR will be posted to |
grahamlangford
changed the title
What does this PR do?
Demo
https://www.loom.com/share/c9f441bfb11146118920b3816a5328e8
Future Work
await tick(),sleep, and mocking out various hooks and components, but nothing seemed to work, the rendered test always had the content hidden and a loading indicator showing. So, future work is to figure that out and actually get a test working to show that the stylesheet link is rendered in the document view. I verified this manually for now.Checklist
src/tsconfig.strictNullChecks.json(if possible)