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

handlebars: Do not format html element attributes but class #8677

Merged
merged 11 commits into from
Jul 23, 2020

Conversation

dcyriller
Copy link
Collaborator

@dcyriller dcyriller commented Jun 30, 2020

Description

This PR stops formatting every html attributes for handlebars. Only class names will be formatted. This was breaking browser rendering and is considered a bug.

This would address #8667 (comment) (100% agreed).

TODO:

  • We'll have to add formatting for style and srcset attributes later (similar to what html does).

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

One will print TextNodes in attributes. One will print other cases (in
ElementNode / Template / Block etc).
TODO: format srcset and style attributes
This reverts commit ef12514.
because it highlights a bug
@fisker
Copy link
Member

fisker commented Jul 2, 2020

Should the attribute case insensitive?

Prettier pr-8677
Playground link

--parser glimmer

Input:

<A CLASS=" foo bar
          "></A>
<A class=" foo bar
          "></A>

Output:

<A CLASS=" foo bar
          " />
<A class="foo bar,[object Object]" />

Prettier pr-8677
Playground link

--parser html

Input:

<A CLASS=" foo bar
          "></A>
<A class=" foo bar
          "></A>

Output:

<a class="foo bar"></a> <a class="foo bar"></a>

@fisker
Copy link
Member

fisker commented Jul 17, 2020

@prettier/core Need review.

@kangax
Copy link
Contributor

kangax commented Jul 23, 2020

@dcyriller @fisker anything I can help with to get this merged? It's blocking the other PR that's also good to go.

@fisker fisker merged commit ed9d399 into prettier:master Jul 23, 2020
@dcyriller dcyriller deleted the prettier-attr branch July 23, 2020 20:53
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants