Add image description text for the audio splice rendering image#366
Add image description text for the audio splice rendering image#366
Conversation
|
Having multiple files seems sub optimal, as the description will likely fall out of sync. Can we instead just throw the relevant information into a |
tidoust
left a comment
There was a problem hiding this comment.
That looks good to me, thanks for addressing the issue!
|
Hadn't seen @marcoscaceres's comment before I reviewed. This PR has the advantage of using a consistent scheme and approach for both images in the spec, but I don't disagree that the description (in both cases) seems excessive and would better be integrated within the main spec. Up to you :) |
marcoscaceres
left a comment
There was a problem hiding this comment.
Let's move what we can into the main document. Having these kind of linked descriptions don't make a lot of sense and can quickly get out of sync.
|
I've removed the additional page, and add a @marcoscaceres @tidoust, please review. Thanks! |
tidoust
left a comment
There was a problem hiding this comment.
Looks good. I guess we should also apply the same logic to the pipeline model diagram afterwards.
|
Sounds good, yes. I've adjusted the pipeline model description in this PR. |
There was a problem hiding this comment.
Pull Request Overview
This PR removes a standalone HTML file describing the Media Source Pipeline Model and integrates its content directly into the main specification document. The changes improve accessibility by replacing external links with inline descriptions and adding proper image descriptions for diagrams.
- Deleted the separate
pipeline_model_description.htmlfile - Embedded the pipeline model description as a collapsible
<details>element within the main specification - Added image descriptions for the audio splice diagram
- Fixed minor formatting issues (trailing whitespace, spelling corrections)
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| pipeline_model_description.html | Removed standalone HTML file containing the pipeline model description |
| media-source-respec.html | Integrated pipeline model description inline, added audio splice image description, fixed spelling and formatting issues |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
media-source-respec.html
Outdated
| Media Source Pipeline Model Diagram | ||
| </figcaption> | ||
| </figure> | ||
| <details class="note"> |
There was a problem hiding this comment.
I think the details should be moved inside the <figcaption> (or ideally removed entirely).
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I added I still think this is excessive and we shouldn't add the |
bf960ce to
622634b
Compare
622634b to
0946914
Compare
marcoscaceres
left a comment
There was a problem hiding this comment.
I'm still very much opposed to this change, on accessibility grounds. Particularly this is violation of:
- Avoid Too Much Content: "Too much information or instruction can be just as much of a hindrance as too little. The goal is to make certain that enough information is provided for the user to accomplish the task without undue confusion or navigation."
- Particularly, having the headings in there was super confusing.
The correct way to handle this is three fold:
- The spec text itself should describe the what the flows are. If there is stuff in the detailed description that is not covered by the spec text, then we should fold that into there.
- I've expanded the figure caption text to describe the what is in the diagram.
- The SVG itself describes what is in the diagram using
desc.
See #312
Preview | Diff