Update design to look more similar to kde.org
ClosedPublic

Authored by ognarb on Mar 6 2019, 12:48 AM.

Details

Summary

This patch mostly change the CSS to look more similar to the one from
kde.org/kontact.kde.org/...

The files common/en/{top-left.jpg,top-right.jpg,kde_logo_bg.png}
are now deprecated and will be removed in KF6.

Screenshots:

Test Plan

Tested with not regenerated documentation and regenerated documentation

Diff Detail

Repository
R238 KDocTools
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9347
Build 9365: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added projects: Frameworks, Documentation. · View Herald TranscriptMar 6 2019, 12:48 AM
Restricted Application added subscribers: kde-doc-english, kde-frameworks-devel. · View Herald Transcript
ognarb requested review of this revision.Mar 6 2019, 12:48 AM
ognarb edited the summary of this revision. (Show Details)Mar 6 2019, 12:50 AM
ognarb added reviewers: VDG, Documentation.
ognarb edited the summary of this revision. (Show Details)Mar 6 2019, 12:58 AM
ognarb edited the test plan for this revision. (Show Details)
ognarb edited the summary of this revision. (Show Details)
aacid added a subscriber: aacid.Mar 6 2019, 6:24 PM

Why changing from jpg to png for the image file ?

broulik added a subscriber: broulik.Mar 6 2019, 6:33 PM

Can't we just go SVG with the icon?

ognarb added a comment.Mar 7 2019, 6:53 AM

Why changing from jpg to png for the image file ?

I only had a png version in my hard drive, but if it's a problem I can convert the png in a jpg.

Can't we just go SVG with the icon?

I tried this first but khtml didn't look like it support svg.

ognarb updated this revision to Diff 53359.Mar 7 2019, 1:13 PM
ognarb edited the test plan for this revision. (Show Details)

Change to jpg and remove now useless files

ognarb updated this revision to Diff 53367.Mar 7 2019, 3:00 PM

Add breeze like style for note/info/warning/tip admonitions and make image responsive

ognarb edited the summary of this revision. (Show Details)Mar 7 2019, 3:04 PM
ognarb updated this revision to Diff 53469.Mar 8 2019, 6:32 PM

Improve display inline media

ognarb updated this revision to Diff 53521.Mar 9 2019, 3:54 PM

Use correct breeze color for important and warning

ognarb added a comment.Mar 9 2019, 3:55 PM

@aacid it is ready to land?

bruns added a subscriber: bruns.Mar 9 2019, 4:57 PM

Why changing from jpg to png for the image file ?

The logo is just 2 colors, and the PNG is smaller than the JPEG, without introducing artifacts. I would definitely prefer a PNG here.

ognarb updated this revision to Diff 53541.Mar 9 2019, 8:44 PM

Use png instead of jpg

ognarb updated this revision to Diff 53542.Mar 9 2019, 8:47 PM

Change png image

bruns added a comment.Mar 9 2019, 8:59 PM

Is it intentional the PNG is 129x129 while the JPEG is 128x128?
Also, the JPEG shold be removed.

ognarb added a comment.Mar 9 2019, 9:26 PM

Is it intentional the PNG is 129x129 while the JPEG is 128x128?

I have no idea why 129x129. I used convert to convert the svg to png, just like I did with the jpg. But I will investigate this issue

Also, the JPEG shold be removed.

I'm not sure, but I think this will be a problem with some documentations where the html wasn't regenerated, because the html wasn't regenerated the image element will link to a deadlink.

ognarb updated this revision to Diff 53544.Mar 9 2019, 9:28 PM

Use a 128x128 png image

ognarb added a comment.Mar 9 2019, 9:29 PM

Lol 128x128 png image 6kb, 129x129 png image 2kb

bruns added a comment.Mar 9 2019, 9:31 PM

Lol 128x128 png image 6kb, 129x129 png image 2kb

The new PNG is fuzzy, thats probably the reason it does not compress well.

Try exporting it with inkscape, and check if the vertical/horizontal edges are pixel grid aligned.

ognarb updated this revision to Diff 53545.Mar 9 2019, 9:42 PM

Use inkscape to generate png

bruns added a comment.Mar 9 2019, 9:44 PM

and now, apply optipng and you are down to 2kB

ognarb updated this revision to Diff 53546.Mar 9 2019, 9:46 PM

Apply optipng. Thanks @bruns I learned something new :D.

@ognarb i don't really have much to say here, my comment was just about a new file being introduced that seemed unnecessary.

I'd say you want @ltoscano for someone that probably knows a bit about docs.kde.org code and maybe someone from VDG to say if it's prettier than before?

rooty added a subscriber: rooty.Mar 10 2019, 10:21 AM

Can you make it use Noto Sans by any chance?

On the graphical side I'm not the most qualified to talk :)
Apart from the VDG, I'm sure that @yurchor can give some hints, as he manages the templates used to generate the PDFs.

From the technical point of view, I wouldn't remove any of the exiting files right now, at least not until KF6. I'd suggest using new names, just to avoid the possible confusion between the old top-kde.jpg and the new top-kde.png.

Apart from the VDG, I'm sure that @yurchor can give some hints, as he manages the templates used to generate the PDFs.

I have no objections. This part is not used for PDFs, btw. ;)

ognarb updated this revision to Diff 53598.Mar 10 2019, 1:35 PM
  • Add deleted image
  • Rename top-kde.png to kde.png
  • Fix strange margin issues for some elements

@rooty The font can be changed in the khelpcenter settings. But it's strange that it doesn't use Plasma settings per default.

yurchor accepted this revision.Mar 10 2019, 2:08 PM
This revision is now accepted and ready to land.Mar 10 2019, 2:08 PM
ltoscano requested changes to this revision.Mar 10 2019, 2:29 PM

Can you please not overwrite top-kde.jpg too, or at least use the same size? Because the change of size may be a problem for existing users (if any, but I consider those files as part of the public interface of KDocTools

This revision now requires changes to proceed.Mar 10 2019, 2:29 PM
ognarb updated this revision to Diff 53603.Mar 10 2019, 4:39 PM

Don't change image size

ltoscano accepted this revision.Mar 10 2019, 8:33 PM

Thanks

This revision is now accepted and ready to land.Mar 10 2019, 8:33 PM
ltoscano requested changes to this revision.Mar 10 2019, 8:34 PM

No, wait a minute. What happens with pages which are not regenerated?

This revision now requires changes to proceed.Mar 10 2019, 8:34 PM

No, wait a minute. What happens with pages which are not regenerated?

They use the old link for the image, also the jpg version instead of the png. I tested with new page and old page and the result is visually the same.

They use the old link for the image, also the jpg version instead of the png. I tested with new page and old page and the result is visually the same.

But do the existing pages use the old link for the image with the new CSS?

They use the old link for the image, also the jpg version instead of the png. I tested with new page and old page and the result is visually the same.

But do the existing pages use the old link for the image with the new CSS?

Yes the link to the css didn't change, only the content. So even the old page links to the new CSS.

In any case, please change the commit message. When this is committed and rechecked in one year from now, the aspect of the linked website may not be relevant anymore and the images are not visible when you run git log.
Please describe what the change is about. https://chris.beams.io/posts/git-commit/

They use the old link for the image, also the jpg version instead of the png. I tested with new page and old page and the result is visually the same.

But do the existing pages use the old link for the image with the new CSS?

Yes the link to the css didn't change, only the content. So even the old page links to the new CSS.

Ok, so let's back one minute, and maybe it was the case in the original review, despite what myself and other people suggested, what if:

  • the changes to the XSL files are reverted
  • top-kde.jpg is rewritten using the same size of the original, and of course same format. No need to add the new kde.png. You may want to add it, but same format, so that we can switch in KF6.

This should ensure no unexpected changes if any user expect the image to be that size, and both newly-generated pages and the old ones will have the same aspect.

Sorry if it was the case in the first review.

The point about the commit message is still valid.

ognarb retitled this revision from Update css to Update design to look more similar to kde.org.Mar 10 2019, 8:48 PM
ognarb edited the summary of this revision. (Show Details)
ognarb edited the test plan for this revision. (Show Details)
ognarb updated this revision to Diff 53616.Mar 10 2019, 8:52 PM

Don't change image link and remove kde.png

ognarb edited the summary of this revision. (Show Details)Mar 10 2019, 8:53 PM
ltoscano accepted this revision.Mar 10 2019, 9:04 PM

Thank you! And sorry for the ping-pong.

This revision is now accepted and ready to land.Mar 10 2019, 9:04 PM

Thank you! And sorry for the ping-pong.

No problems ;) Thanks for the review.

This revision was automatically updated to reflect the committed changes.
kossebau added inline comments.
common/en/kde-docs.css
252 ↗(On Diff #53367)

Seems there is no such CSS property font-height? Was this meant to be e.g. font-size or line-height?