zmarkdown: fix image & one smiley

[artragis]

Since v27 deployment we got some feedback, this PR fixes two of them :

• we cannot use :P anymore
• when we use image figures are not created. (fix Add quizz to zmarkdown and rebber stringification #420)

[dvbmgr]

Should'nt we just convert emoticon to lower case in /packages/remark-emoticons/src/index.js:36` instead ?

[artragis]

perhaps. What do you think @vhf

[vhf]

I usually don't consider smileys to be case sensitive.

[artragis]

ok, so I will modify remark-emoticons.

[artragis]

I think this is mergeable @vhf? @davbaumgartner

[vhf]

Sorry, this is as broken as the previous PRs. I'll let you find out why this time. Pointing out specific issues or explaining what's wrong didn't help in your past PRs.

[artragis]

@vhf: WHat I see here:

• PR title is OK
• tests are complete : block image, inline image, transposition in html & latex
• tests are what we expect : block image gives caption when alt title is given
• snapshots are consistent.

I don't see any point.

[artragis]

so @vhf what can we do? Those bugs are annoying for zds members et I don't understand what you require me to do.

[vhf]

What would be really annoying is adding more bugs. Please add tests dedicated to the case insensitive smileys.

[artragis]

Like this one? 9e37b2f#diff-37df776abe73b7b3095b1f81808ffe4eR79

[vhf]

Yes, a bit like this one except:

• Without bugs (as I said on many of your previous PRs, you need to read the git diff before committing, this way you can check that what you commit is what you actually want to commit and you don't end up pushing broken things),
• In its own test case (as we do whenever we discover a bug: we add a test case to make sure there will be no regression, and we name this test case with whatever it is actually testing).

[artragis]

Without bugs (as I said on many of your previous PRs, you need to read the git diff before committing, this way you can check that what you commit is what you actually want to commit and you don't end up pushing broken things),

As I said in the previous PRs, I DO read the diff.

But as I said in previous PRs snapshot are really hard to read. When you see a bug it's far more easier to tell me you've seen it.

AS this pr is little, I managed to get your point after reading it 5 times with focus on the snapshot. If you found out the error at the first glance, I really don't get the point of not telling it.

I will spli the test to extract the :p versus :P

[vhf]

That was not the only bug. Looking for the smiley you got, then again with both lowercase and uppercase versions cannot work. What if the configuration contains a smiley with mixed case?

I fixed this bug and fixed the tests (snapshots are not well suited here) in here: #246

Regarding your other commit, wrapping an image in a figure, I'd like to understand how it should behave before reviewing the code. The two test cases are not enough to get the intent or to check whether this will behave as expected.

Should all images become figures? Only the ones with an alt text and without a caption? Only images that are either inline or the only element of a block? Understanding your goal will also help deciding where this code should be, where it's now or as part of remark-captions.

[artragis]

Regarding your other commit, wrapping an image in a figure, I'd like to understand how it should behave before reviewing the code. The two test cases are not enough to get the intent or to check whether this will behave as expected.

if you have an image which is not inline and not included inside a figure you must transform it as a figure.

For example:

Hello world

![legend](image)

becomes

<p>Hello world</p>
<figure>
<img src="image" alt=""/>
<figcaption>legend</figcaption>
</figure>

Nota : the blank "alt" is not mandatory, it is an old behaviour which was not the best (alt text should always be here)

If you have a legend, you get :

Hello

![alt](url)
FIgure: Legend

<p>Hello world</p>
<figure>
<img src="image" alt="alt"/>
<figcaption>Legend</figcaption>
</figure>

This case is the one treated by remark-captions

THen for inline images (emoticons and all images that has type inlineImage, nothing must change.

[vhf]

Ok, I think I get it. Last question:

Should:

![foo](http://bar)

![](http://bar)
Figure: foo

give A:

<p><figure><img src="http://bar" alt="foo"><figcaption>foo</figcaption></figure></p>

<p><figure><img src="http://bar"><figcaption>foo</figcaption></figure></p>

or B:

<figure><img src="http://bar" alt="foo"><figcaption>foo</figcaption></figure>

<p><figure><img src="http://bar"><figcaption>foo</figcaption></figure></p>

[artragis]

I would say "C" :

<figure><img src="http://bar" alt="foo"><figcaption>foo</figcaption></figure>

<figure><img src="http://bar"><figcaption>foo</figcaption></figure>

I mean : remark-captions doesn't wrap figures between <p> and </p>

[vhf]

I mean : remark-captions doesn't wrap figures between <p> and </p>

It does. I don't think HTML allows figures to be in ps though. [edit] Yep, that's not allowed.

[artragis]

When I use figures in zds website I have no wrapping p

[vhf]

You are looking at what your browser parsed. Your browser respects the HTML spec. You should look at the HTML sent by the webserver to your browser.

[artragis]

True engough, thanks for the tip.

[vhf]

See the <p></p> right above <figure> on your screenshot? That's your browser immediately closing the <p> opened here: <p><figure> because figure is not allowed in p. Seeing this made me think figure might not be allowed in p, that's why I checked and reported this above.