Bug: Pre() FTs are rendered incorrectly

phact commented 1 month ago

Here's a minimum reproducible example:

from fasthtml.fastapp import *

app,rt = fast_app()

@rt("/")
def get():
    return Div(
        Pre(Code("No spaces!"), data_prefix="1"),
        Pre(Code("No spaces!"), data_prefix="1"),
    )

serve()

$ curl http://0.0.0.0:5001/
<!doctype html></!doctype>
<html>
  <head>
    <title>FastHTML page</title>
    <meta charset="utf-8"></meta>
    <meta name="viewport" content="width=device-width, initial-scale=1, viewport-fit=cover"></meta>
    <script src="https://unpkg.com/htmx.org@next/dist/htmx.min.js"></script>
    <script src="https://cdn.jsdelivr.net/gh/answerdotai/surreal@1.3.0/surreal.js"></script>
    <script src="https://cdn.jsdelivr.net/gh/gnat/css-scope-inline@main/script.js"></script>
    <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@picocss/pico@latest/css/pico.min.css">
    <style>:root { --pico-font-size: 100%; }</style>
  </head>
  <body>
<div>
  <pre data-prefix="1">
    <code>No spaces!</code>
  </pre>
  <pre data-prefix="1">
    <code>No spaces!</code>
  </pre>
</div>
  </body>
</html>

For correctness it should have rendered:

  <pre data-prefix="1"><code>No spaces!</code></pre>
  <pre data-prefix="1"><code>No spaces!</code></pre>

insted of

  <pre data-prefix="1">
    <code>No spaces!</code>
  </pre>
  <pre data-prefix="1">
    <code>No spaces!</code>
  </pre>

We want:

No spaces!

No spaces!

not:

    No spaces!

    No spaces!

phact commented 1 month ago

I took a stab at a fix upstream https://github.com/fastai/fastcore/pull/594

This would render the html inline and leave formatting to dev_tools on the browser (which I find does a good job). Can anyone think of any downsides to doing this?

If folks think this is a good approach it would just be a matter of adding inline=True to https://github.com/AnswerDotAI/fasthtml/blob/main/fasthtml/core.py#L269 and https://github.com/AnswerDotAI/fasthtml/blob/main/fasthtml/core.py#L228

I'm happy to create the PR if desired but I'll hold off for now until I get some feedback.

jph00 commented 1 month ago

Thanks for the PR, which is now merged. But I wonder if we should also deploy a more "proper" fix which has a list of inline elements, and doesn't reformat those regardless of config?

yawaramin commented 1 month ago

I fixed this in my library by line-breaking the output HTML only when printing an attribute: https://github.com/yawaramin/dream-html/commit/b51af0723bf5b94c62bb5c1634b423f9b883a145

jph00 commented 1 month ago

I fixed this in my library by line-breaking the output HTML only when printing an attribute: yawaramin/dream-html@b51af07

Very nice! I'm a huge fan of dream-html BTW -- definitely a big inspiration for us :D

phact commented 1 month ago

Ok took another stab at the fastcore improvement, this time with inline_tags https://github.com/fastai/fastcore/pull/605

fix here would be

HTMLResponse(to_xml(resp, inline_tags=['pre']), headers=http_hdrs)

phact commented 4 weeks ago

@jph00 could you have a look? Thanks in advance!

AnswerDotAI / fasthtml

Bug: Pre() FTs are rendered incorrectly #235