PolymerElements / paper-dropdown-menu

A Material Design browser select element
https://www.webcomponents.org/element/PolymerElements/paper-dropdown-menu
61 stars 107 forks source link

Update paper-dropdown-menu-shared-styles.html #209

Closed mkazlauskas closed 7 years ago

mkazlauskas commented 7 years ago
  1. Fixes error

    I assume it has to do with the way css is parsed in the polyfill.

  2. Fixes textContent label with nested slots. See new test for example.

notwaldorf commented 7 years ago

/cc @valdrinkoshi

valdrinkoshi commented 7 years ago

@mkazlauskas could you provide more details on how to repro that error? I cannot repro it in the demo page on safari, chrome or firefox.

Regarding 2), that looks like a feature request not related to the 2.0 preview, you have the same behavior in paper-dropdown-menu 1.x. I would open a separate issue where to discuss about this, then open a PR if we reach consensus on the need of this 👌

mkazlauskas commented 7 years ago

@valdrinkoshi I wrote test for it.

$ git clone git@github.com:firmfirm/paper-dropdown-menu.git
$ cd paper-dropdown-menu && git checkout 2.0-preview && bower install

In paper-dropdown-menu.html replace line #369 with what it originally was:

value = selectedItem.label || selectedItem.getAttribute('label') || selectedItem.textContent.trim();

Run tests: $ polymer test

valdrinkoshi commented 7 years ago

As per the Contributing guidelines, we require to first open an issue before opening a PR.

About 1), I can't repro it, can you open an issue giving more details on it? e.g. which browser are you using, version..

About 2), why should the paper-dropdown-menu search on the shadow dom of another element? I can't understand exactly the use case you're trying to accomodate. paper-dropdown-menu already provides different ways for you to tell it what's the label: set the label property or attribute, or fallback to the textContent. Let's discuss this on an issue ;)

Closing this.

mkazlauskas commented 7 years ago

I actually found more details about css error and posted it under polymer repo: https://github.com/Polymer/polymer/issues/4118 although it may belong in shadycss polyfill.