twostraws / Ignite

A static site generator for Swift developers.
MIT License
986 stars 34 forks source link

Placement of <script> after </body> seems to render in browsers but is recommended to be before </body> #25

Closed steve-h closed 4 weeks ago

steve-h commented 4 weeks ago

I am just starting to look at Ignite and my Nova editor gave an error/warning about the .js files added outside the HTML body. They are supposed to be in the head section or at the end of the body to speed the page render.

I see there is another issue to make conditional inclusions of .js only when needed for each page. I am adding this here as a stepping stone to at least get conformance.

public struct HTML: PageElement {
   ...
 public func render(context: PublishingContext) -> String {
        var output = "<!doctype html>"
        output += "<html lang=\"\(context.site.language.rawValue)\" data-bs-theme=\"light\"\(attributes.description)>"
        output += head?.render(context: context) ?? ""
        output += body?.render(context: context) ?? ""
        output += Script(file: "/js/bootstrap.bundle.min.js").render(context: context)

        if context.site.syntaxHighlighters.isEmpty == false {
            output += Script(file: "/js/syntax-highlighting.js").render(context: context)
        }

        // Activate tooltips if there are any.
        if output.contains(#"data-bs-toggle="tooltip""#) {
            output += Script(code: """
            const tooltipTriggerList = document.querySelectorAll('[data-bs-toggle="tooltip"]')
            const tooltipList = [...tooltipTriggerList].map(tooltipTriggerEl => new bootstrap.Tooltip(tooltipTriggerEl))
            """).render(context: context)
        }

        output += "</html>"

        return output
    }

Some of the code should go into the body render function.

twostraws commented 4 weeks ago

Thank you! Would you be happy to open a PR to correct this, or would you prefer me to do it? I think it would be a matter of moving the extra script generation code into the Body type.

twostraws commented 4 weeks ago

This is fixed in #26.