phoenixframework / phoenix_live_view

Rich, real-time user experiences with server-rendered HTML
https://hex.pm/packages/phoenix_live_view
MIT License
6.22k stars 931 forks source link

HTMLFormatter adds unwanted whitespace to `contenteditable` elements #1951

Closed simoncocking closed 2 years ago

simoncocking commented 2 years ago

Environment

Actual behavior

When using a contenteditable element, whitespace within the tag is significant. However the HEEX formatter insists on breaking the tag onto multiple lines, which introduces unwanted whitespace into the element's content:

~H"""
<div contenteditable>The content</div>
"""

becomes

~H"""
<div contenteditable>
  The content
</div>
"""

Expected behaviour

I think we need some way to override the formatter's behaviour in some way for some cases - perhaps a similar mechanism that allows us to tell Dialyzer to ignore issues in the following function?

josevalim commented 2 years ago

We could detect the contenteditable attribute and treat it the same way with treat code/pre? /cc @feliperenan

xtian commented 2 years ago

@josevalim It would be ideal if this behavior was not hard-coded but could be opted-in to at the template level (perhaps via a template directive or config comment) since adding whitespace also affects the :empty CSS pseudo-class.

Even though the CSS selectors spec was changed some time ago to ignore whitespace for :empty, every browser still implements the whitespace-significant version.

Here's an example use-case using Tailwind:

<div class="empty:hidden" phx-click="lv:clear-flash" phx-value-key="error"><%= get_flash(@conn, :error) %></div>
JonRowe commented 2 years ago

This also affects <textarea> e.g. these are not the same:

<textarea><%= @content %></textarea>

<textarea>
  <%= @content %>
</textarea>

As the formatter shouldn't be making changes to the meaning of code/markup, I'd suggest that tag "breaking" (e.g. split tags onto new lines) should be done on an allow list basis, e.g. known safe combinations, or prehaps not at all?

josevalim commented 2 years ago

We should definitely treat textarea as code/pre.

I'd suggest that tag "breaking" (e.g. split tags onto new lines) should be done on an allow list basis

Disagree. Most of the time we want it but we should provide mechanisms to disable if hardcoding exceptions is not enough.

josevalim commented 2 years ago

Worst case scenario we add phx-no-break attribute, which is kept by the formatter but not emitted in the actual HTML.

SteffenDE commented 2 years ago

Hi there,

the formatter is awesome, but I just tried this change and noted that it does not solve the problem for custom components, e.g.

<.textarea ...>My content</.textarea>

being formatted as

<.textarea ...>
  My content
</.textarea>

and the component rendering into a textarea like this:

<textarea class="my custom classes" {@extra_assigns}><%= render_slot(@inner_block) %></textarea>

Something like phx-no-break seems to be necessary.

Should I create a separate issue for this?


Despite that, commit bba75d29911502bc24ee5d70ac31664b6973bf2f seems to have introduced some weird formatting issues for me:

image
josevalim commented 2 years ago

Can you open up two issues? One for the regression and another for phx-no-break? A pull request for phx-no-break is welcome. It should be a matter of adding it alongside contenteditable and skipping its emission in the engine.

SteffenDE commented 2 years ago

Done, #1975 and #1976.

logicmason commented 1 year ago

I am also encountering this issue on spans:

<span class="underline decoration-secondary decoration-2 font-extrabold">Check</span> Messages

is being transformed into:

<span class="underline decoration-secondary decoration-2 font-extrabold">
  Check
</span>
Messages

The original code underlines "Check", but the formatted code underlines "Check ", including the space between the two words.

aiwaiwa commented 1 year ago

A perfect html formatter to me would be the one that adds no new lines unless the author of the file added a new line, then it would push the spaces according to alignment rules. That would solve all these rendering issues.

ahacking commented 1 year ago

I have the exact same problem as @logicmason where most of my spans get reformatted and underline styling includes trailing space due to the formatting. I say most because infuriatingly it depends on the length of the classes attribute as to whether a span element is reformatted or not.

I would suggest NOT messing with spans ever.

I would also suggest generalizing the textarea logic and provide a common escape hatch to never format under any circumstances whenever phx-no-break is present, irrespective of the element type.