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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: fix and simplify stylesheets for the manuals, fix nrd bug #295847

Merged
merged 4 commits into from
Mar 16, 2024

Conversation

DanielSidhion
Copy link
Member

@DanielSidhion DanielSidhion commented Mar 14, 2024

Description of changes

As discussed in NixOS/nixos-homepage#1251, the Nixpkgs and NixOS manuals are now being rendered as-is on the new website. This "breaks" the manuals, because previously we relied on a very complex post-processing step (which was done on the homepage's repository) to get the manuals as we're all used to. This included using the stylesheets from the old homepage.

Since we can't rely on that anymore, I went through the trouble of understanding the steps that were being done in the old homepage, figuring out the final stylesheet rules, and then translating those rules back to the manuals as generated by Nixpkgs. This was done to avoid having to do all those complex steps anymore, I just want to simplify things.

As part of this simplification:

  • We don't need an overrides.css stylesheet anymore, so I merged all stylesheets into one.
  • I discovered the source of a bug that existed in the manuals as generated by Nixpkgs, but interestingly didn't exist after post-processing on the old homepage (likely because the old homepage did some XML-related processing which probably fixed the bug). This bug is now fixed in nixos-render-docs.
  • Ported the anchorjs script from the old homepage (and an extra one to call it), which is responsible for adding the links to the headers in the manuals. The scripts were previously added to the end of the manual, but since nixos-render-docs doesn't support that, I adapted them to work with the scripts at the top of the manual.

Note: the new stylesheets are not absolutely 1:1 with the old stylesheets, which relied on more stuff from the old homepage that I don't want to port over (mainly the custom fonts). In addition to that, I modified the stylesheets a bit to address this comment: NixOS/nixos-homepage#1251 (comment)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 馃憤 reaction to pull requests you find important.

@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Mar 14, 2024

This is very good. Maybe throw in a Nix logo for good measure like in NixOS/nix#9870

I'd merge as is.

@DanielSidhion DanielSidhion marked this pull request as ready for review March 14, 2024 15:00
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed this in the docs team meeting today:

Looks like there's a minor problem with examples (in the NixOS manual here):
image

Whereas before it was
image

Also make sure to check the options listing, it's a separate page that perhaps uses separate CSS classes.


And some potential future improvements

  • Less rounded corners

  • No shadow for code samples

  • Improve header font size, especially in lib, might need some adjustments to font-size, or switching it to rem instead of percentages (cc @hsjobeki)

    image

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-03-14-documentation-team-meeting-notes-113/41462/1

Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything I mentioned above has been addressed, however I spotted a new problem: The options page has odd left/right margins:

image

The empty space on the right is only visible if I scroll. Seems better on smaller screens.

If you have the time to fix this that would be great, but otherwise I can also merge already :)

@DanielSidhion
Copy link
Member Author

I tried to look into it for a fixed amount of time, but couldn't find what's causing that. When the browser has enough width, the scroll bar doesn't show up, but as I reduced the browser's width, I could reproduce this behaviour.

My guess is that there is some content in this page that is causing this behaviour, but I can't find the content because there is just too much there. I looked into <code> boxes that have long lines, but in those cases the <code> box has a scroll bar as expected, so I don't know how they could be contributing to this behaviour. It's possible there is some other element that I couldn't find.

@infinisil
Copy link
Member

Alright let's not worry about it for now then, this is already much better than the current state, so let's merge!

@infinisil infinisil merged commit a1581a3 into NixOS:master Mar 16, 2024
21 checks passed
wegank pushed a commit to wegank/nix-darwin that referenced this pull request Mar 17, 2024
@DanielSidhion DanielSidhion deleted the fix-manuals branch March 19, 2024 02:14
@DanielSidhion DanielSidhion added the backport release-23.11 Backport PR automatically label Mar 21, 2024
Copy link
Contributor

Successfully created backport PR for release-23.11:

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

Successfully merging this pull request may close these issues.

None yet

7 participants