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

Double headings when using labels #2

Closed
SR4ven opened this issue Jan 19, 2020 · 6 comments
Closed

Double headings when using labels #2

SR4ven opened this issue Jan 19, 2020 · 6 comments
Assignees
Labels
pending new release Fixed, but pending a new release

Comments

@SR4ven
Copy link
Contributor

SR4ven commented Jan 19, 2020

Using labels, results in duplicated headings as shown in the picture below.
Looks like the cause for this is that the theme only removes the first child h1, but in this example the first child is an empty span.

grafik

.. _connections:

===========
Connections
===========

Connection objects implement :class:`ITargetConnection <boofuzz.ITargetConnection>`. Available options include
:class:`SocketConnection <boofuzz.SocketConnection>` and :class:`SerialConnection <boofuzz.SerialConnection>`.

Examples::

    tcp_connection = SocketConnection(host='127.0.0.1', port=17971)
    udp_connection = SocketConnection(host='127.0.0.1', port=17971, proto='udp')
    udp_connection_2_way = SocketConnection(host='127.0.0.1', port=17971, proto='udp', bind=('127.0.0.1', 17972)
    udp_broadcast = SocketConnection(host='127.0.0.1', port=17971, proto='udp', bind=('127.0.0.1', 17972),
                                     udp_broadcast=True)
    raw_layer_2 = (host='lo', proto='raw-l2')
    raw_layer_2 = (host='lo', proto='raw-l2',
                   l2_dst='\\xFF\\xFF\\xFF\\xFF\\xFF\\xFF', ethernet_proto=socket_connection.ETH_P_IP)
    raw_layer_3 = (host='lo', proto='raw-l3')
@autophagy autophagy self-assigned this Feb 18, 2020
@tomaarsen
Copy link
Contributor

The issue here is that the title is explicitly placed with the main-content just below it, like so:

<div id="main-content-header">
<h1>{{ title }}</h1>
</div>
<div id="main-content">
{% block body %}{% endblock %}
</div>

Then, the first h1 in the main-content is hidden with this:

// Hides the first header of the documentation, as it is instead put into
// the main-content-header
h1:first-child {
display: none;
}

However, this only works if the first-child of #main-content is indeed that h1. In my case, there was an invisible span before the h1, which meant that this CSS rule was never applied.

I resolved this in my fork https://github.com/tomaarsen/nltk_theme by removing #main-content-header, and removing the rule that removes the first h1 from the main content. Then, to align everything correctly again, I simply added margin-top to the h1:first-of-type of #main-content, which applies to the first h1 under #main-content.
See the commit here: tomaarsen@620aaba
Do note that the commit also does some other things, i.e. some renaming, but that can be ignored.

Another fix which might work is simply updating h1:first-child from

// Hides the first header of the documentation, as it is instead put into
// the main-content-header
h1:first-child {
display: none;
}

with h1:first-of-type. That said, I prefer my solution of not arbitrarily deleting the first h1, and just using the main-content as it is generated.

Also, @autophagy, because I haven't explicitly told you this yet: Well done on this theme. It's very slick, and I like it a lot! You may have noticed already, but we've started using it for https://www.github.com/nltk/nltk under https://www.nltk.org/.

  • Tom Aarsen

@autophagy
Copy link
Owner

@tomaarsen Firstly, thank you for the complement! I had never intended for others to really use this (hence my self-indulgent naming of 'docs' as 'seonu' and stuff 😅) but its awesome you were able to get some use out of it! I have been keeping an eye on your PR for NTLK and was about to reach out to you to ask whether you'd be comfortable with me cherrypicking/adapting some improvements from your fork 😄

If you have any painpoints regarding developing on Insegel, let me know! I would like to improve the developer docs (as in, add any at all) and maybe remove the whole SCSS -> CSS build cycle since its a bit dated. But let me know if any issues you ran into that might improve the dev experience!

@tomaarsen
Copy link
Contributor

@autophagy Hah, I was already wondering why the docs had that name. I even googled it, to see if I could find the meaning (Is it meant to mean "Season"?)
And of course! You can take my changes as you see fit. One more thing, I decided on crediting you in the setup/PyPI stuff like so: https://github.com/tomaarsen/nltk_theme/blob/master/setup.py
I didn't update the author_email, but I reckon that maybe that would be wise? If you have other comments on the crediting or anything, let me know.

Regarding painpoints, I have one big suggestion, and it revolves around a problem that users might have. In Sphinx, it's either really tricky or impossible to add your own CSS files. I tried many different methods, and none seemed to work. I could get the CSS added to the build, but the layout.html never included a reference to this CSS file. So, the only way to modify the CSS of your website is to override an existing CSS file, which does get imported from the layout.html.

The solution to this, inspired from an issue from the Alabaster theme (sphinx-doc/alabaster#78), is to add an empty custom.css file to the project, which is imported from the layout.html. Users can them add a custom.css file in their html_static_path, which overrides this (empty) file, and their custom CSS rules will be used in the website.
This allows for a way to modify the CSS without having to fork the theme, but I ended up wanting to change more things, so I felt like a fork was most proper. See tomaarsen@633fb82 for my commit on this.

Beyond that, it took me a bit of digging to figure out exactly what information was taken for the source URL at the bottom of the page. If you don't fill in your conf.py correctly, then this will (I believe?) simply link to https://www.github.com/None/None/....

One other nitpicky thing - on mobile, the "hamburger" menu and the table of contents are placed at the bottom of the page, which was somewhat unintuitive to me. I ended up changing this, but I recognise that this could also just be a design decision. I changed this in tomaarsen@9a13777.

Oh, and I enjoyed the workflow with SCSS -> CSS, and working with Grunt. I had attempted to use SCSS before, but it never really worked out, and I also hadn't used Grunt before.

@autophagy
Copy link
Owner

@tomaarsen For a lot of my personal projects I pull words from Old English, and 'seonu' means sinews or nerves (I think the intention was that docs are the sinews that hold a project together).

In terms of attributions, feel free to change the author_email away. I'm not sure how many people actually use that for reaching out to a package maintainer, but I suppose it makes sense now that they're different packages.

Ah, thank you for the heads up with the custom.css file! This has always been a bit of a problem, I had no idea alabaster had solved it like that. I will definitely add this, since like you said, the other option is maintaining a fork for just CSS changes which feels a little overkill.

Yeah, the source URL at the bottom is a little unintelligent - no checking if its actually set, assumes you use Github and not Gitlab/sourcehut/darcshub etc. I'm not even sure I remember where the commit information comes from, it might be from ReadTheDocs (which makes it less flexible for being deployed elsewhere).

I think the hamburger placement was just an afterthought and, I agree, is more intuitive at the top than the bottom.

Thank you for the feedback! ✨ I'll try and cherry-pick the commits from your fork into here, but in case of a conflict I will add you as an author and myself as a co-author to the commits.

@tomaarsen
Copy link
Contributor

Sounds great! Let me know if there's any issues, etc.

autophagy added a commit that referenced this issue Oct 22, 2021
This should address #2

Co-authored-by: Tom Aarsen <[email protected]>
@autophagy autophagy added the pending new release Fixed, but pending a new release label Oct 22, 2021
@autophagy
Copy link
Owner

@SR4ven This should now be fixed with version 1.3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending new release Fixed, but pending a new release
Projects
None yet
Development

No branches or pull requests

3 participants