Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New custom CSS for https://qgis.org/api/ #2954

Merged
merged 12 commits into from
Apr 8, 2016
Merged

New custom CSS for https://qgis.org/api/ #2954

merged 12 commits into from
Apr 8, 2016

Conversation

tomchadwin
Copy link
Contributor

Discussed at http://lists.osgeo.org/pipermail/qgis-developer/2016-March/042070.html.

  • new file /doc/api_custom.css containing changes-only styles
  • /cmake_templates/Doxyfile.in changed:
    • HTML_EXTRA_STYLESHEET = ../doc/api_custom.css added
    • HTML_COLORSTYLE_HUE changed to 113 (QGIS logo green)

Tested with a local Windows copy of Doxygen running on QGIS master, changing only the settings above from default. Output tested on IE11 and Chrome/Win.

Sample screenshots:

screenshot 2016-03-30 10 45 22

screenshot 2016-03-30 10 50 06

screenshot 2016-03-30 10 50 45

Testing on other platforms required. Testing also required of how the actual https://qgis.org/api/ Doxygen process is run.

@m-kuhn
Copy link
Member

m-kuhn commented Mar 30, 2016

For the version bump change line 31 in doc/CMakeLists.txt to

FIND_PACKAGE(Doxygen 1.8.xx REQUIRED)

(replace xx with the version number you need)

@tomchadwin
Copy link
Contributor Author

@m-kuhn Thanks. Is the advice to upgrade to the minimum version required, or to current stable?

This is the minimum version required - happy to bump it higher if
preferred
@mbernasocchi
Copy link
Member

CSS and screenshots look sensible to me

@m-kuhn
Copy link
Member

m-kuhn commented Mar 30, 2016

On 03/30/2016 12:57 PM, Tom Chadwin wrote:

@m-kuhn https://github.com/m-kuhn Thanks. Is the advice to upgrade
to the minimum version required, or to current stable?

minimum required

@tomchadwin
Copy link
Contributor Author

@m-kuhn Done - bumped to 1.8.2.

@m-kuhn
Copy link
Member

m-kuhn commented Mar 31, 2016

I already like it!

@rduivenvoorde @anitagraser @NathanW2 to ping some random people who might be interested :)

@anitagraser
Copy link
Member

I like the color changes and it also seems like you added some extra paddings/margins so the content looks less tight. Is the CSS responsive?

@tomchadwin
Copy link
Contributor Author

Not responsive, no, but can be done later, although there is quite a lot of use of px, not %. Can be incrementally implemented, though.

@tomchadwin
Copy link
Contributor Author

I tested this with Doxygen current stable and minimal configuration - no doxyfile. I'll try to run 1.8.2 with this PR's doxyfile tomorrow and report back.

@tomchadwin
Copy link
Contributor Author

I've now run Doxygen 1.8.2 locally with the master Doxyfile, manually editing it only to replace some @PATH_VARIABLES@ with relative paths from the Doxyfile location:

@CMAKE_CURRENT_BINARY_DIR@ = ../cmake
@CMAKE_SOURCE_DIR@ = ..

It seems to have worked, so I am moderately confident this will work in production. Obviously, I'd be happier if someone could test it running as part of cmake (or however it works), so that the @PATH_VARIABLES@ are dynamically set. But it's looking pretty good.

I'm not very happy with the appearance of the tabbed navigation when there is more than one tab row, but I still think overall it's enough of an improvement. The tags CSS can always be improved later.

@tomchadwin
Copy link
Contributor Author

I hadn't tested with graphviz/dot, but have now. It works, though no changed colours are used, so I guess the Doxyfile's HTML_COLORSTYLE_HUE is not being passed to dot. Never mind - it works.

@m-kuhn
Copy link
Member

m-kuhn commented Apr 6, 2016

It probably works - and if it doesn't there will not be much trouble.

I am in favor of merging it. Any objections?

@m-kuhn m-kuhn merged commit e3bfec0 into qgis:master Apr 8, 2016
@m-kuhn
Copy link
Member

m-kuhn commented Apr 8, 2016

Let the audience decide

Thanks @tomchadwin 👍

@tomchadwin
Copy link
Contributor Author

Thanks, @m-kuhn and all.

-------- Original message --------
From: Matthias Kuhn notifications@github.com
Date: 08/04/2016 13:13 (GMT+00:00)
To: qgis/QGIS QGIS@noreply.github.com
Cc: Tom Chadwin tom.chadwin@nnpa.org.uk
Subject: Re: [qgis/QGIS] New custom CSS for https://qgis.org/api/ (#2954)

Let the audience decide

Thanks @tomchadwinhttps://github.com/tomchadwin [:+1:]


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHubhttps://github.com//pull/2954#issuecomment-207404899

Tom Chadwin, ICT Manager
Telephone: 01434 611530 Mob:
Web: www.northumberlandnationalpark.org.ukhttp://www.northumberlandnationalpark.org.uk/

IMPORTANT NOTICE - Disclaimer - This communication is from Northumberland National Park Authority (NNPA).The Authority’s head office and principal place of business is Eastburn, South Park, Hexham, Northumberland, NE46 1BS, United Kingdom. If you are not the intended recipient(s) please note that any form of disclosure, distribution, copying or use of this communication or the information in it or in any attachments is strictly prohibited and may be unlawful. If you have received this communication in error, please delete the email and destroy any copies of it. Any views or opinions presented are solely those of the author and do not necessarily represent those of NNPA.Contractors or potential contractors are reminded that a formal Order or Contract is needed for NNPA to be bound by any offer or acceptance of terms for the supply of goods or services Although this email and any attachments are believed to be free of any virus or other defects which might affect any computer or IT system into which they are received, no responsibility is accepted by the NNPA for any loss or damage arising in any way from the receipt or use thereof. Computer systems of this Authority may be monitored and communications carried out on them recorded, to secure the effective operation of the system and for other lawful purpose.

@tomchadwin
Copy link
Contributor Author

Any idea when this will update the live API docs site?

@m-kuhn
Copy link
Member

m-kuhn commented Apr 11, 2016

Looks live to me http://qgis.org/api/modules.html

@tomchadwin
Copy link
Contributor Author

Hrm. Only one aspect has worked - the change in the HUE variable. However, Doxygen has not bumped to 1.8.2, so the extra CSS has not been pulled in. Any ideas why?

@m-kuhn
Copy link
Member

m-kuhn commented Apr 11, 2016

@rduivenvoorde do you know about the infrastructure the api doc builder runs on?

@jef-n
Copy link
Member

jef-n commented Apr 11, 2016

The api docs are taken from the nightlies. Debian unstable has doxygen 1.8.11 (should be >1.8.2).

@jef-n
Copy link
Member

jef-n commented Apr 11, 2016

fixed in 4815ebd

@tomchadwin
Copy link
Contributor Author

@jef-n Apologies. I misread 1.8.11 as 1.8.1.1, which is what we were on before. Many thanks for the fix.

@NathanW2
Copy link
Member

Nice work Tom.

On Tue, Apr 12, 2016 at 5:32 AM, Tom Chadwin notifications@github.com
wrote:

@jef-n https://github.com/jef-n Apologies. I misread 1.8.11 as 1.8.1.1,
which is what we were on before. Many thanks for the fix.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#2954 (comment)

@tomchadwin
Copy link
Contributor Author

Thanks, Nathan. Still not showing the extra stylesheet - will check tomorrow.

@tomchadwin
Copy link
Contributor Author

Still no extra stylesheet, though I've only checked visually. Will view source when I am at my laptop.

@tomchadwin
Copy link
Contributor Author

Confirmed - still no additional CSS file linked in HTML. If it has run since @jef-n's change, have to check the Doxygen logs to see what happened. Are they accessible to the likes of me?

@jef-n
Copy link
Member

jef-n commented Apr 13, 2016

The api doc on site is from abda90f. The current nightly is still building.

@tomchadwin
Copy link
Contributor Author

Great. It's all there now. Very much needs responsive work for mobile, though. Will look when I can.

@m-kuhn
Copy link
Member

m-kuhn commented Apr 14, 2016

Thank you @tomchadwin and @jef-n it
It's much more pleasant to view now. Looking forward to seeing more of this!

@tomchadwin
Copy link
Contributor Author

Yes, thanks for the fix, @jef-n.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants