render: fix center image test #3880
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
| let result = markdown_to_html(text, None); | ||
| assert_eq!( | ||
| result, | ||
| "<img src=\"https://img.shields.io/crates/v/clap.svg\" alt=\"\" align=\"center\">\n" | ||
| "<p align=\"center\"><img src=\"https://img.shields.io/crates/v/clap.svg\" alt=\"\"></p>\n" |
There was a problem hiding this comment.
aren't these cases already sufficiently tested by the text_alignment test above?
There was a problem hiding this comment.
The text_alignment test guarantees that the align attribute can be set for <h1> and <h5> HTML tags. By being explicit in this other test, we make sure that centering images will always be supported by catching any possible future regression. Testing is cheap, finding regressions is hard.
There was a problem hiding this comment.
shouldn't it be renamed to paragraph_alignment then? I guess the <p> doesn't care whether a <img> is inside or not 😅
There was a problem hiding this comment.
You are right that the <p> does not care currently. But maybe in a future release the <p> will care because of implementation changes. The usecase I want to cover is centering images in README.md on crates.io, so I make sure that this particular usecase is tested and will continue to work in the future.
It also as a side effect documents how to center images on crates.io for users.
|
@bors r+ |
|
📌 Commit 34191ae has been approved by |
Turbo87
added
A-backend ⚙️
C-internal 🔧
|
☀️ Test successful - checks-actions |
The image test is actually not representative and will not center the image. This fix helps users discover how to center images and proves that they will be centered on crates.io.
Note: I am the original author of this testcase from PR #3862