The example is way too long, and should be split up as individual examples for each value instead

[Pomax]

MDN URL

https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/preserveAspectRatio

What specific section or headline is this issue about?

examples

What information was incorrect, unhelpful, or incomplete?

The example is way too long, and should be split up as individual examples for each value instead

What did you expect to see?

examples to show what each value does, inlined with the documentation for that value.

Do you have any supporting links, references, or citations?

No response

Do you have anything more you want to share?

It is currently impossible to tell which image in the "final image" relates to which part of the (incredibly scrolled-off-page) code example.

MDN metadata

Page report details

• Folder: en-us/web/svg/attribute/preserveaspectratio
• MDN URL: https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/preserveAspectRatio
• GitHub URL: https://github.com/mdn/content/blob/main/files/en-us/web/svg/attribute/preserveaspectratio/index.md
• Last commit: b4f9982
• Document last modified: 2023-03-06T23:38:10.000Z

[hamishwillee]

I agree - the example is very monolithic. This could be split up into parts as you suggest. Are you interested in taking this on?

In addition, I would also reorder the headings like other docs - Description, Syntax, and then Examples. The examples list should be changed to a definition list.

[Pomax]

Unfortunately I probably won't have time to sit down for it given that various other projects that also demand my attention =(

[wbamberg]

I agree - the example is very monolithic. This could be split up into parts as you suggest. Are you interested in taking this on?

In addition, I would also reorder the headings like other docs - Description, Syntax, and then Examples. The examples list should be changed to a definition list.

The ordering in this page is consistent with the other SVG pages though: for no good reason SVG uses a different ordering from the rest of MDN. So this would be yet another rabbit hole on MDN.

[Pomax]

No matter how deep the rabbit hole, a cup of dirt at a time will eventually fill it =)

[Jenna59]

@Pomax @hamishwillee This looks like it could be a good first issue for me. Is anyone currently working on it? It says there's no assignee. Thanks!

[teoli2003]

The issue is yours!

[Jenna59]

I think I'll style it as Hamishwillee suggested to bring it more in line with MDN, as a whole.

[hamishwillee]

@Jenna59 I wouldn't. While it would be a good idea, this would be better done across the whole of SVG as a project.

What I would do is split the example into sections.

[Jenna59]

@hamishwillee I'm completely new to open source, so apologies if this is a silly idea: I was thinking that since all of the SVG docs are like that, I could possibly go through the other ones and bring them up to date after finishing this ticket. I just don't know if it would be worthwhile to do that, or how exactly I would raise it as an issue.

[teoli2003]

Revamping all the examples of SVG would be a multi-month project. I also believe that start contributing by splitting this example but keeping the ordering used in SVG.

[hamishwillee]

@Jenna59 It's a kind thought, but as @teoli2003 says, there's a lot of work in converting SVG to match the other formats. We'd want to add some scripting to ease this, and to validate our changes before starting, and bring enough people on board to make sure that the docs don't get stuck in an intermediate state.

Maybe something for the future! SVG is much neglected so plenty of other less drastic things to do now.

In the meantime, splitting the example up is a good thing to do.

[Jenna59]

Sounds good! Thanks for the feedback.

[Jenna59]

@wbamberg Hi! Do you want to take a look before I make a pull request? I split the examples and made them larger since they were really tiny. https://github.com/Jenna59/content/blob/Jenna59-issue26352/files/en-us/web/svg/attribute/preserveaspectratio/index.md

[wbamberg]

I've had a quick look and it seems pretty great! There's one error where you have:

preserveAspectRatio="xMaxYMid meet width height"

...which I think ought to be:

preserveAspectRatio="xMaxYMid meet"

...otherwise it seems to work perfectly.

I do wonder though if breaking it up as much as this is going too far, and loses the connection between each of the examples. In the original example you can see from the output that examples are implicitly in groups of three, except the last one:

xMidYMid meet (width > height)
xMinYMid meet (width > height)
xMaxYMid meet (width > height)

xMidYMin slice (width > height)
xMidYMid slice (width > height)
xMidYMax slice (width > height)

xMidYMin meet (height > width)
xMidYMid meet (height > width)
xMidYMax meet (height > width)

xMinYMid slice (height > width)
xMidYMid slice (height > width)
xMaxYMid slice (height > width)

none

...so I wonder if it is worth keeping this grouping, so we end up with 5 examples instead of 13. We could give them titles like "meet (width > height)" and even have a bit of prose, like:

This example shows the use of meet when the element's width is greater than its height. It presents three variations, with three different alignment values: xMidYMid, xMinYMid, and xMaxYMid.

What do you think?

[Jenna59]

That makes more sense to me, too :) There's a lot of repetitive code in there that could be taken out that way.

[hamishwillee]

Agree with those comments ^^^. Even as it is, is much better than currently.

[Jenna59]

@wbamberg & @hamishwillee I made the changes and it looks really good! The only thing I noticed was a bit of a glitch with the Slice (height > width) example on a large screen - the svg looks like it's wider than the rectangle, even though they're the same size. Do you think it's ready for a PR? Also, before I do the PR, should I merge the changes from MDN/content into my forked repo, so that it's up to date?

[wbamberg]

@Jenna59 , I think it's ready for a PR :).

Also, before I do the PR, should I merge the changes from MDN/content into my forked repo, so that it's up to date?

It's probably a good idea to do this, because if someone has made conflicting changes to the same file, you'll have to fix that sooner or later, so might as well make a mergeable PR to begin with.

[Pomax]

Would it make sense to give each example a guidetext/plain paragraph heading? Reading through the PR it looks like right now it's been split up, but without anything to say what each block of code is showing off.

[Jenna59]

@Pomax I can't seem to figure out what's going on with my PR request issue right now. I tried cloning my fork of the MDN content repo to local again and it doesn't want to run... There was a line added in the sections about how each example showed 'meet' and 'slice' and the attributes that could be used with them.