kwebio / kweb-core

A Kotlin web framework
https://docs.kweb.io/book
GNU Lesser General Public License v3.0
969 stars 57 forks source link

Can't remove/replace viewport meta "K_head" #404

Closed alexpopa95 closed 1 year ago

alexpopa95 commented 1 year ago

Hello, I have a strange behavior in which I set my custom head viewport meta but there still is the default tag with id="K_head" (see screenshot below)

My configuration

fun main() {
    Kweb(
        port = 8080,
        debug = false,
        plugins = listOf(
            StaticFilesPlugin(ResourceFolder("static"), "/static"),
            ViewportPlugin(),
            fomanticUIPlugin
        )
    ) {
        doc.head.new {
            // other head tags
            // I tried adding my meta here also, nothing changes
        }
        doc.body.new {
            // routing
        }
    }
}

Screenshot

meta_k_head

Is this behavior normal?

Derek52 commented 1 year ago

Try

doc.head {

}

doc.body {

}
alexpopa95 commented 1 year ago

@Derek52 Thank you for the suggestion. Tried it but I have the same issue.

sanity commented 1 year ago

I think that's a bug in Kweb, HtmlDocumentSupplier.createDocTemplate() is adding the first meta tag, which I don't think belongs there because that should be under the user's control. It's possible that I added this back in the early days.

Also, it's setting <meta>'s id to K_head which is already being used for the <head> element.

I think the fix is to remove that code so it's not appending the <meta>, but this may break people's pages so it will require a clear explanation in the release notes.

Thoughts?

sanity commented 1 year ago

This is the code that will be removed: https://github.com/kwebio/kweb-core/blob/432e9aeba243ac1b5f875bdcf53fa75cc55a9054/src/main/kotlin/kweb/html/HtmlDocumentSupplier.kt#L27-L30

sanity commented 1 year ago

Ok, 1.3.0 is out - hopefully it doesn't break a lot of people's websites but it shouldn't - from what I can tell the values it was using were the defaults anyway.

alexpopa95 commented 1 year ago

Thank you @sanity. I can confirm it is now working as expected. I tried in various scenarios and without the meta tag the website behaves the same as before (in my case and I think in the case of others)

I would have an improvement to make for the ViewportPlugin. Right now I am using a custom plugin similar to the one Kweb provides but with more customizations as this:

class ViewportPlugin(
    private val width: ViewportWidth = ViewportWidth.DeviceWidth,
    private val initialScale: Double = 1.0,
    private val maximumScale: Double = 1.0,
    private val userScalable: Boolean = false
) : KwebPlugin() {
    override fun decorate(doc: Document) {
        doc.head().appendElement("meta")
            .attr("name", "viewport")
            .attr(
                "content",
                "width=${width.text}" +
                        ", initial-scale=$initialScale" +
                        ", maximum-scale=$maximumScale" +
                        if (!userScalable) ", user-scalable=no" else ""
            )
    }
}

@sanity Do you think it could be done for the project? May help others needing these configurations. I could create a PR for it.

sanity commented 1 year ago

Yes, that would be great if you want to improve the ViewportPlugin - I'd be happy to merge a pull request.

sanity commented 1 year ago

Actually, I'm starting to wonder if a Viewport component might be a better way to do this. I created this plugin before Kweb supported server-side rendering, so a plugin was the only option.

If you just want to stick with improving the plugin that's fine - but if you're interested in doing it as a component I think that would be a better solution for the long-term. Thoughts?

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.