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

Hugo TODO #28479

Open
XhmikosR opened this issue Mar 13, 2019 · 26 comments
Open

Hugo TODO #28479

XhmikosR opened this issue Mar 13, 2019 · 26 comments

Comments

@XhmikosR
Copy link
Member

XhmikosR commented Mar 13, 2019

  1. make sure our folder structure is the proper one for what we want to achieve. Also see if we can make folder nesting level smaller, maybe by using permalinks or something. For content and static.
  2. can we generate SRI hashes for existent files on the fly? For bootstrap.min.css, bootstrap.*.js, popper.js/1.14.7/umd/popper.min.js. This would simplify things as we could remove the release-sri script
  3. get image width/height with Hugo when possible (examples images, homepage)
  4. maybe use Hugo Extended version for docs' CSS? This is now done
  5. bep's suggestion:

    Re.

    {{< partial "callout-warning-color-assistive-technologies.md" >}}
    

    The partial above uses the partial func to load, parse and execute templates. This has its limitations and is possibly ineffective.
    I would recommend that you store your callouts (and other content files you want to include) in a so-called headless bundle -- and load them via .GetPage. This way you can use markdown + shortcodes etc.

  6. bep's suggestion:

    Ref. {{ block "header" . }} in your single.html template.
    Blocks only make sense inside base template(s) (i.e. baseof.html), which I cannot see. I would highly recommend that you restructure your templates to use them. It removes some repetition.

  7. Fix
    <a class="d-block text-decoration-none" href="../{{ urlize .title }}/">
  8. Add a body class like layout-{{layout}} page-{{kind}}-{{title}}{{ with .Params.body_class }} {{ . }}{{ end }}
@XhmikosR
Copy link
Member Author

@bep: can you give me an example how to get the SRI hashes https://github.com/twbs/bootstrap/blob/master/config.yml#L49?

The files do exist locally, we just use the cdn link for the doc snippets.

Also, is there any way to get a sha384 hash instead of sha256? I know that 256 should be enough but that's what we've been using from day 1 and also we suggest on srihash.org, BootstrapCDN etc.

Thanks :)

@bep
Copy link
Contributor

bep commented Mar 15, 2019

@XhmikosR have you looked in the documentation? Note that I'm very busy at the moment (also with Hugo), so I would appreciate if you could take general Hugo questions either with 1) the docs or 2) the forum

It is sha256 or sha512 (or md5).

@XhmikosR
Copy link
Member Author

All right, I'll try again based on the docs alone.

Have you thought about exposing sha384 too, though? I can make an issue upstream if you want.

@XhmikosR
Copy link
Member Author

Unfortunately, I can't make it work, like the previous time I tried. Maybe I'm doing it wrong or it just can't work with our setup.

{{ $style := resources.Get "docs/4.3/dist/css/bootstrap.css" | resources.Fingerprint }}
{{ $secureCss := $style | resources.Fingerprint "sha256" }}
<link href="{{ $secureCss.Permalink }}" integrity="{{ $secureCss.Data.Integrity }}" rel="stylesheet">

Maybe someone else has a solution, we'll see.

@budparr
Copy link

budparr commented Mar 15, 2019

Curious why you're running Fingerprint twice? Also, I think resources.Get is looking for your CSS file relative to the assets directory, so you may need to change your path. I'm happy to pull the repo down and fool around with it if that would be helpful.

@XhmikosR
Copy link
Member Author

@budparr: please do :)

The path to that file is relative to the /static dir; unfortunately since we can't copy our root dist folder where we need to with Hugo, we need to do this in a manual step.

So, if you do npm run docs-serve you will get the directory structure with the dist folder copied in /static/docs/4.3/dist/.

Thanks in advance for any help!

@budparr
Copy link

budparr commented Mar 15, 2019

@XhmikosR As I mentioned earlier, Hugo is looking for those files in the assets directory, yet you're pushing them to the static folder where Hugo can't read them. You're not using Hugo to process your CSS, so you'll have to read the files from that directory.

  1. Change the assets directory setting in the config to match your dist folder (it does not appear you are otherwise using it).

    So change assetDir: "site/assets" to assetDir: "dist"

  2. Then, read the dist directory, looping through them to apply the Fingerprint function:

{{/*  Identify the directory */}}
  {{ $path := "./dist/css" }}
  {{ $files := readDir $path }}
{{/*  Range through the files */}}
  {{ range $files }}
    {{/*  You may not need to do this, but the conditional allows you to determine what you want to apss through */}}
    {{ if in .Name "bootstrap" }}  
    {{/* Do your work on each file  */}}
      {{ $file := resources.Get (printf "css/%s" .Name) | resources.Fingerprint }}          
      <link href="{{ $file.Permalink | relURL }}" rel="stylesheet" integrity="{{ $file.Data.Integrity }}" crossorigin="anonymous">      
    {{ end }}
  {{ end }}

I did a very quick test on this on your repo, but you may want to adjust to your needs.

@XhmikosR
Copy link
Member Author

XhmikosR commented Mar 17, 2019

@budparr: thanks for looking into this. Unfortunately, I cannot change the assetDir to that. We have our other assets in site/static/.

If I could specify the output folder for the assetDir values, I could then just include the dist folder too. But this isn't supported.

EDIT:
Actually I think I misunderstood what you said. Let me try your suggestion rn.

EDIT2:
Alright, this works but 1) the paths are wrong; it's now /css/ which is not what we need it to be (/docs/4.3/dist, and 2) we don't need Hugo to actually touch these files, we just want to get their SRI without changing the output folder or the files. Maybe that's not possible?

@budparr
Copy link

budparr commented Apr 4, 2019

Hey, @XhmikosR I only saw your first edit. Did you make any progress on this?

@bep
Copy link
Contributor

bep commented Apr 5, 2019

{{ $js := resources.Get "/docs/4.3/dist/bootstrap.js" }}
{{ $fingerprinted := $js | fingerprint }}

{{ $fingerprinted.Data.Integrity }} // The SRI string
{{ $js.RelPermalink }} // The original path, e.g. /docs/4.3/dist/bootstrap.js
{{ $fingerprinted.RelPermalink }} //  /docs/4.3/dist/bootstrap.xxxxSHA256xxxx.js

Note that for the above to work, these must live inside assetDir.

Another approach that may work better for you is to create a "headless bundle" for these (which would live inside the contentDir).

You would then do something ala:

{{ $bundle := .Site.GetPage "myassets" }}
{{ $js := $bundle.Resources.GetMatch "**/4.3/**/bootstrap.js" }}
{{ $fingerprinted := $js | fingerprint }}
{{ $fingerprinted.Data.Integrity }} // The SRI string
{{ $js.RelPermalink }} // The original path, e.g. /docs/4.3/dist/bootstrap.js
{{ $fingerprinted.RelPermalink }} //  /docs/4.3/dist/bootstrap.xxxxSHA256xxxx.js

@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 5, 2019

@budparr: no progress :/

@bep I just gave this a go and it works, sort of :)

Basically I just changed the dir we copy the dist folder to site/assets and then I manipulate the dist files as usual.

  1. This works, but we are missing the .map files and generally any file we don't reference in the templates.
  2. traditionally we use sha384 hashes everywhere, BootstrapCDN, srihash.org etc. Could this be exposed by Hugo assuming we make this to work?
  3. If we go with this approach, it would make sense to use the fingerprint values everywhere instead of us computing them and outputting them in config.yml. So maybe the headless bundle approach is indeed more appropriate here.

Not sure which solution is the best, this or the headless bundle approach, that's something you guys might be able to answer :)

Here's my current WIP branch https://github.com/twbs/bootstrap/compare/master-xmr-hugo

I'm gonna try the headless bundle approach next.

@bep
Copy link
Contributor

bep commented Apr 5, 2019

Re sha384 -- see gohugoio/hugo#5815 -- I will get that in the (big!) release on Monday.

With map files etc. in play, I think a "regular leaf bundle" will do the work, as map etc. files will just be copied to destination.

@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 9, 2019

It seems I'm having trouble creating the headless bundle, could be the content/layouts organization again.

@budparr: could you try my branch and then see if you can make the headless bundle work, please?

Thanks in advance!

@budparr
Copy link

budparr commented Apr 10, 2019

Hey, @XhmikosR I've been out for a conference. Happy to dig into this, though it might take me a couple of days to clear my plate first. Is the branch master-xmr-hugo?

@XhmikosR
Copy link
Member Author

Yup, no pressure :)

@XhmikosR
Copy link
Member Author

XhmikosR commented Jan 1, 2020

@budparr @regisphilibert maybe one of you guys could help us out with tweking the layouts/partials? :)

@budparr
Copy link

budparr commented Jan 3, 2020

@XhmikosR We're happy to help. I wasn't able to solve the issue above. Is that the issue specifically we can help with, or others?

@XhmikosR
Copy link
Member Author

XhmikosR commented Jan 3, 2020

@budparr I have a TODO list in my OP #28479 (comment)

The layouts issue is a big one and I couldn't fix it either. But if we could solve any of the above, it would help us a lot :) Also, since many people might use our docs for learning purposes, I want us to make sure we do things the right way.

@budparr
Copy link

budparr commented Jan 3, 2020

Okay. Well @regisphilibert and I will review and see what we can get done.

@XhmikosR
Copy link
Member Author

XhmikosR commented Jan 4, 2020

Thank you guys, I really appreciate it!

@XhmikosR
Copy link
Member Author

XhmikosR commented Jan 14, 2020

I added one more TODO:

  1. Add a body class like layout-{{layout}} page-{{kind}}-{{title}}{{ with .Params.body_class }} {{ . }}{{ end }}

@regisphilibert
Copy link
Contributor

regisphilibert commented Jan 14, 2020

Hello. I'd love to help you in the persective of checking those "to dos". I have a couple of questions:

On 1.

Yes, you directory structure can be completely independent of your URL structure. It's hard for me to guess what your ideal content directory structure would be (though I suppose losing 4.3 could help of replacing it by current if the project must build serveral version), but if you provide me with a directory structure I could rearange the files accordingly while maintaining current URL structure.

On 2.

Hugo is capable of producing fingerprints on resources only if they're part of the assets of the project, meaning living in the assets directory which is currently set to site/assets. The resource you want fingerprinted happens to be in the static directory. So we cannot have both live in static and assets...
In order to fingerprint the bootstrap stylesheet, we'd have to move it or mount it as an asset along its brand images.

On 3.

Same as above, Hugo is capable of retrieving the width and height of a given image only if given file is part of the assets directory or a Page Bundle (being a page resource.

On 4.

You are currently using a shortcode to load partials from within the content files's markdown. I can see that 23 times out of 24 times this shortcode is used, it is to call of the partial files prefixed callout- which only contain markdown. Björn Erik suggestion is for you to actually use proper content files to handle those "callouts" and for this move them into the content directory rather the layout directory. They would take the form of a headless page bundle, meaning a directory of content files (could be /content/callouts/ which would not produce any published pages or section.
Then we could introduce a shortcode which fetches such content files and produce their rendered markdown.

He is my proposal:

As it seems this behaviour is only used with the callout "shortcodes", we improve the callout shortcode by adding a second argument (on top of the "css class" one) which would point to this "content file filename" under the newly created content directory.
If second argument is set and we find a matching "content file", we print its rendered markup, if not we print the content of the shortcode as it is currenlty used.

From:

{{< callout danger >}}
{{< partial "callout-danger-async-methods.md" >}}
{{< /callout >}}

To:

{{< callout danger "danger-async-methods" />}}

The following on the other hand would remain unchanged:

{{< callout info >}}
Note that content should not be larger than the height of the image.
{{< /callout >}}

On 6.

No problem I think.

On 7.

Current the url is build from the page title which binds title and url together.
It seems the logic here uses an array of "sections" assigned as Front Matter param to build a side menu.
There is several better approach:

  1. Keep that logic but reference the content file in the array as relative to the current page:
sections:
  - title: Form control
    page: form-control
    description: Style textual inputs and textareas with support for multiple states.
  - title: Select
    page: select
    description: Improve browser default select elements with a custom initial appearance.

Then in the code we can use .GetPage with the items's .page value. .to retrieve the page and its .RelPermalink.
2. Use Hugo Menu system. The drawback is that each concerned page will need to point to the menu in its own Front Matter.

On 8.

No problem, will be easier once 6. is implemented though.

@XhmikosR
Copy link
Member Author

XhmikosR commented Jun 10, 2020

@regisphilibert thanks for the thorough reply and I'm so sorry for the late reply!

On 1.

Yes, you directory structure can be completely independent of your URL structure. It's hard for me to guess what your ideal content directory structure would be (though I suppose losing 4.3 could help of replacing it by current if the project must build serveral version), but if you provide me with a directory structure I could rearange the files accordingly while maintaining current URL structure.

We'd like to keep the version in the built files, but we don't mind dropping it from the repo. The opposite, dropping the version from the source folders would simplify things a lot, because we wouldn't need to git mv the folders every time we change the major/minor version. It's something I've been trying to do but without full success.

On 2.

I guess it's OK to drop this for now. We'll just keep running run the script we have before a release which updates the SRI hashes in config.yml and we commit the changes. We can of course revisit this later because it would simplify our release process :)

On 3.

Same as above, Hugo is capable of retrieving the width and height of a given image only if given file is part of the assets directory or a Page Bundle (being a page resource.

Currently we have the images in the static folder because we don't process them, but it's low priority so let's leave this for now :)

On 4/5.

Your solution does seem the proper one, I don't have anything to say apart from feel free to make a PR when you have time :)

On 7.

Your first suggestion seems the simplest one at this point. I didn't use Hugo's menu system because of the limitation of having to specifying everything in the frontmatter; having the menu in one place is pretty handy.

On 8.

No problem, will be easier once 6. is implemented though.

Sounds good.


To sum it up: we are now preparing to release 5.0.0-alpha1, hopefully in a week or so. I'm trying to see what's left. If we could sort the layouts/folder hierarchy/docs version removal/ the other should be easier to tackle.

No pressure, I really appreciate all the help you five us! And if it wasn't for @bep's work, we wouldn't be able to make the switch in the first place :)

Now we just need to dot the i's and cross the t's and all this work will be live soon :)

PS. You decide what issue to tackle first and just CC me on any PRs you make
PPS. After we soft out some of the issues I have here, I will close this issue and open separate issues so that it's easier and cleaner to track things

EDIT: forgot to mention the other thing that I'd like to finish is #29283. It works, but it needs some kind of error checking.

@mdo
Copy link
Member

mdo commented Sep 6, 2021

@XhmikosR Is this still something to keep open or can we close?

@XhmikosR
Copy link
Member Author

XhmikosR commented Sep 8, 2021

We still have issues that we haven't fixed. Maybe some day someone will jump in to help us with these :)

@sdkdeepa
Copy link

Hello! I would like to contribute to this as it is still open. Please let me know

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

No branches or pull requests

6 participants