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

layout.html should not be bundled with the polymer.html import. #942

Closed
philipwalton opened this issue Nov 18, 2014 · 6 comments
Closed
Labels

Comments

@philipwalton
Copy link

While I fully support the goal of making it easier for developers who use Polymer to do layout in a simple, sane way, I think the way it's currently being done contradicts pretty much all the best-practices Web Components are trying to promote.

When you include the polymer.html import, you get a global stylesheet containing global attribute rules that all use the /deep/ and universal selectors. In other words, they match every single element inside <html>, even elements inside a shadow boundary.

This means that the following attributes are essentially "claimed" by Polymer: flex, block, hidden, relative, fit, and segment. If anyone else uses them, there may be unintended consequences (which, again, is exactly what Web Components are supposed to help avoid). There are also several other attributes, which, if used in conjunction with the wrong attribute, will have the same problem: horizontal, vertical, inline, auto, none, etc.

These are all pretty common words, so there's an extremely high probability that other developers will use them without realizing they come with style baggage. In fact, that's how I discovered layout.html in the first place. I was trying to debug a conflict.

The very first sentence on the Polymer website reads:

Web Components usher in a new era of web development based on encapsulated and interoperable custom elements...

Encapsulation

Web Components are supposed to be pro-modular and anti-globals. The main selling point of Shadow DOM is style encapsulation, and yet Polymer, out of the box, breaks that with no way to opt out.

Interoperability

One thing that makes Web Components superior to all existing UI solutions is the promise of interoperability. The point is you can import a custom element, and, regardless of whether it was made with X-Tags or Polymer or with just JavaScript, it's supposed to work and not conflict with the existing elements on the page.

However, if I'm building a site with X-Tag and I import an element made with Polymer, I'm going to get layout.html and it's going to potentially conflict with every other element I use.

Even if I try to avoid this problem by just including polymer.js (instead of polymer.html), I will still run into this issue if I include anyone else's element that imports polymer.html anywhere in the dependency graph.

How it should be done

There is no reason layout.html needs to be bundled with Polymer. It's just a stylesheet, and like every other stylesheet, it can and should be included at the developer's discretion.

Another option is to make it a standalone layout element that other elements can compose or extend from to get this styling. At least this way there won't be surprises.

@mitranim
Copy link

polymer.html does nothing but import layout.html and polymer.js. If someone finds they don't want layout, they can just import polymer.js instead.

@philipwalton
Copy link
Author

polymer.html does nothing but import layout.html and polymer.js. If someone finds they don't want layout, they can just import polymer.js instead.

@mitranim unfortunately, it's not as simple as that. As I state above, if any element I use imports polymer.html (anywhere in the dependency graph) then I get layout.html. And, as I'm sure you know, pretty much every custom element in existence imports polymer.html.

@mitranim
Copy link

The problem is actually even worse. Polymer requires that you load it via rel=import, and refuses to initialise if you just include the polymer.js file. I'm optimising a website for fast page loads, bundling all css and js and inlining it on the page to avoid extra network requests. Including an import for polymer.html adds 50-100ms to page load time and delays the loading of a few other assets by the same interval. It basically increases page load time by 50%.

It seems Polymer devs think that forcing XHR is okay. I find this rather grotesque.

Edit. Nevermind, I was being incredibly stupid: forgot to change <link> to <script> when loading polymer.js. Apparently you can also inline it with no ill effects. Nevertheless, the layout should definitely be optional.

@rchedrese-va
Copy link

@philipwalton layout.html is causing conflicts with the google-analytics components. If you use the segment attribute of <google-analytics-query> it applies styles from layout.html. I also can't find any documentation on the segment layout attribute, does it need to be there? would prefixing the layout attributes be an option?

@philipwalton
Copy link
Author

Ping.

Polymer team, as @rchedrese-va points out, layout.html is already causing conflicts with existing Google Web Components elements. Please consider removing layout.html or making it opt-in in some way.

@rchedrese-va, the only way I can think of to deal with this problem right now is to manually override the styles imposed by layout.html. Adding something like the following to the head of your page should do it:

html /deep/ google-analytics[segment] {
  display: flex;
  position: static;
  margin: 0;
  padding: 0;
  background-color: transparent;
  -webkit-box-shadow: none;
  box-shadow: none;
  border-radius: 0;
}

You'll also have to make sure to update that code if either layout.html styles change or if the styles in the <google-analytics-chart> element change. Yes, this is gross, but hopefully it helps to illustrate why layout.html is a bad idea.

@tjsavage tjsavage added the 0.5 label May 21, 2015
@tjsavage
Copy link
Contributor

Closing this issue due to age and the release of version 1 of Polymer - please feel free to re-open if this is incorrect.

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

No branches or pull requests

4 participants