697840 - Better presentation of images in KB articles

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task, P4)

Description

[Verdi [:verdi] Best to slack me]

Currently we often need to add multiple breaks before and after images in sumo articles to get them to display correctly. Also when we use images inline in a sentence they've recently been shifted upward a few (5?) pixels. 

We should have styling for images that is consistent and works without a lot of trial and error for KB editors. Using "*:" for images in a list of steps worked nicely at one point (I never new you could do that - Michelle showed it to me) but it doesn't anymore.

Comment 1

[Kadir Topal [:atopal]]

Michael, is this something you'd still like to have?

Comment 2

[Verdi [:verdi] Best to slack me]

Yes! We should talk about what would make sense.

Comment 3

[Kadir Topal [:atopal]]

Adding top and bottom paddings to images should be fairly straight forward as far as I can tell. But of course adding them now would require us to edit all the articles and remove the line breaks.

Also, I'm not sure sue what you mean with 'Using "*:" for images in a list of steps worked nicely at one point'

Anyway, you can try it out, by adding this to your user stylesheet:

.wiki-image {
padding: 20px 0
}

And if you are okay with the results, it's a quick fix in the code

Comment 4

[Verdi [:verdi] Best to slack me]

Actually 5px will fix the issue and prevent images with two breaks, <br><br>, from looking bad. And we can fix the problem with images used in a sentence with vertical align. I think this would be a win:

.wiki-image {
padding: 5px 0;
vertical-align: middle;
}

Comment 5

[Kadir Topal [:atopal]]

In a pull request:
https://github.com/mozilla/kitsune/pull/1528

Please put this on staging first, and let Michael give feedback before moving it to production.

Comment 6

[Ricky Rosario [:rrosario, :r1cky]]

Landed on master:
https://github.com/mozilla/kitsune/commit/021a81af0aac78125fb943f00a9f212327208471

This is on stage now. :verdi, please verify? I added a test image here:
https://support.allizom.org/en-US/kb/how-to-enable-java-if-its-been-blocked

You'll need to upload more and/or edit some articles to see the effects since stage doesn't have the images from prod.

Comment 7

[Verdi [:verdi] Best to slack me]

This looks good to me. Here's the article I used to come up with the suggestion - https://support.allizom.org/en-US/kb/testing-images-1

Comment 8

[Ricky Rosario [:rrosario, :r1cky]]

Deployed to prod now. Thanks Verdi and Kadir!

Comment 9

[Scoobidiver (away)]

Awesome! Icons and labels are now aligned in https://support.mozilla.org/fr/kb/obtenir-assistance-communaute

Comment 10

[Scoobidiver (away)]

It's not fixed for the mobile website. See https://support.mozilla.org/fr/kb/obtenir-assistance-communaute?&mobile=1

Comment 11

[Ricky Rosario [:rrosario, :r1cky]]

The bug didn't mention anything about mobile. Please file a new bug if we need to make changes for mobile. Thanks

Comment 12

[Scoobidiver (away)]

(In reply to Ricky Rosario [:rrosario, :r1cky] from comment #11)
> The bug didn't mention anything about mobile.
It was implicit.

> Please file a new bug if we need to make changes for mobile.
It's bug 902875.

Comment 13

[Ricky Rosario [:rrosario, :r1cky]]

(In reply to Scoobidiver from comment #12)
> (In reply to Ricky Rosario [:rrosario, :r1cky] from comment #11)
> > The bug didn't mention anything about mobile.
> It was implicit.

Well, Verdi filed this and verified it. Kadir implemented it. Clearly, neither of them thought it was implicit ;).

Comment 14

[Will Kahn-Greene [:willkg] ET needinfo? me]

Re-adding [needsverify] flag. I'm not sure why it was removed--it shouldn't have been.

Comment 15

[Scoobidiver (away)]

(In reply to Will Kahn-Greene [:willkg] from comment #14)
> Re-adding [needsverify] flag. I'm not sure why it was removed--it shouldn't
> have been.
I removed it now that it has been verified. It's the way the verifyme keyword works.

Comment 16

[Will Kahn-Greene [:willkg] ET needinfo? me]

This isn't the verifyme keyword. It's a flag we use to mark bugs that need verification by a non-dev so we know we need to account for that extra step.

Please don't remove it in the future for SUMO bugs.