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

[WIP] Fix handling of inline and preformatted tags #41

Closed
wants to merge 11 commits into from
Closed

[WIP] Fix handling of inline and preformatted tags #41

wants to merge 11 commits into from

Conversation

jarrodldavis
Copy link
Contributor

@jarrodldavis jarrodldavis commented May 25, 2019

I'm opening this pull request early to get feedback on the tests I added before finishing the fixes I had in mind.

The following seem like they will be hard to do until a string option type is provided by Prettier (see prettier/prettier#6151):

  • Allow additive customization of tags that are considered preformatted – mainly to allow for configuring components without overriding configuration of vanilla HTML tags
  • Allow additive customization of tags that are considered inline – mainly to allow for configuring components without overriding configuration of vanilla HTML tags

@@ -0,0 +1,4 @@
<a>
<MyIcon />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should we handle block elements inside inline elements? Should they wrap as normal?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I think wrap as normal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so it looks like Prettier handles this differently (with htmlWhitespaceSensitivity set to strict or css): even with child block tags, it doesn't break immediately after the opening tag nor immediately before the closing tag.

Here are some examples

Coincidentally, that behavior is the easiest way to implement the fixes for inline elements in this plugin. Here's the updated behavior

@jamesbirtles
Copy link
Contributor

Looks good so far 👍

leny added a commit to becodeorg/kareble that referenced this pull request May 27, 2019
- Split various element tests into separate tests for inline and block
  tags
- Add tests for various scenarios involving inline tags, including
  inside other tags and alongside text nodes
- Add test for not reformatting text inside `<pre>` tags
Add definitions for all elements, preformatted elements, and inline
elements (as specified by TypeScript and Mozilla Developer Network).
Add options for specifying what tags should be treated as inline and
preformatted elements.
- Collect inline element tags (as defined by the `svelteInlineElements`
  option) into inline groups when printing children.
- When printing children of inline elements, don't add surrounding lines
  around the child contents
Don't break immediately after the opening tag or immediately before the
closing tag, even with block element children.
@bdadam
Copy link

bdadam commented Sep 12, 2019

What do you think, will this PR also solve #58 or is that a different issue?

@lukeed
Copy link
Member

lukeed commented Nov 9, 2019

This is a great PR. What else needs to be done?

@jarrodldavis
Copy link
Contributor Author

Hey sorry, it's been a while since I've looked at this. From my checklist, it seems I was stuck on making the configuration of inline/preformatted tags as flexible as possible since Prettier didn't have a proper string type for options, but it appears that has since been fixed: prettier/prettier#6219

Also, maybe more tests? 😬

@awmottaz
Copy link

@jarrodldavis can I help get this PR across the finish line?

@jarrodldavis
Copy link
Contributor Author

@awmottaz Feel free to fork/copy over my branch and make your own PR based on this one. I don't have the time right now to finish this, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants