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

Deserialization of HTML attributes in Polymer.PropertyAccessors mixin #4689

Closed
2 of 6 tasks
alim opened this issue Jun 15, 2017 · 6 comments
Closed
2 of 6 tasks

Deserialization of HTML attributes in Polymer.PropertyAccessors mixin #4689

alim opened this issue Jun 15, 2017 · 6 comments

Comments

@alim
Copy link

alim commented Jun 15, 2017

Description

First off let me say I am new to Polymer and JavaScript is not my strong suit. So, hopefully, I am not too far off base here.

We are using Polymer 2.0 and paper-input-elements in conjunction with a Rails 5.1 web application. In particular we are using the paper-dropdown-menu in an Rails view template, along with paper-input elements. Both elements are at version 2.0. The paper-input elements work fine. The paper-dropdown-menu displays as disabled in both Chrome (59.0.3071.86) and Firefox (53.0.3).

Steps to Reproduce

  1. Create a Rails html.erb file with the sample paper-dropdown-menu code
    in it as follows:

    <paper-dropdown-menu
      label="Dinosaurs">
        <paper-listbox slot="dropdown-content">
          <paper-item>allosaurus</paper-item>
          <paper-item>brontosaurus</paper-item>
          <paper-item>carcharodontosaurus</paper-item>
        </paper-listbox>
    </paper-dropdown-menu>
    
  2. Use either default disabled property for the paper-dropdown-menu
    element or try to set it to false directly as follows:

    <paper-dropdown-menu
      label="Dinosaurs"
      disabled="false">
        <paper-listbox slot="dropdown-content">
          <paper-item>allosaurus</paper-item>
          <paper-item>brontosaurus</paper-item>
          <paper-item>carcharodontosaurus</paper-item>
        </paper-listbox>
    </paper-dropdown-menu>
    
  3. Run the Rails application to display the page.

Expected Results

We expect to see the paper-dropdown-menu menu element displayed and
enabled so we can select one of the dropdown options.

Actual Results

The paper-dropdown-menu element is displayed but "grayed" out and
does not respond to mouse or keyboard input.

Diagnosis and Proposed Fix

In debugging the problem, I found the following:

  1. The paper-dropdown-menu element includes a mixin that sets the
    default value for the disabled attribute to false. The mixin is
    Polymer.IronControlState.

  2. When the page is displayed this default value is deserialized and the
    default value is converted to true. This deserialization takes place
    in Polymer.PropertyAccessors. The code that deserializes the
    Boolean value is here
    and shown below:

    case Boolean:
      outValue = (value !== null);
      break;
    
  3. I believe unless the value is null, it will always converts the Boolean
    value to true. This conversion causes the paper-dropdown-menu
    to be disabled.

  4. I would propose the following change that is shown below and in a
    forked copy of the repository on the deserialize branch
    here.
    The suggested change is below:

    case Boolean:
     if (value === null) {
       outValue = false;
     } else if (value.toLowerCase() === "true") {
       outValue = true;
     } else if (value.toLowerCase() === "false") {
       outValue = false;
     } else {
       outValue = false;
     }
     break;
    

If you think I captured the solution correctly or you have suggestions
for any changes, I would be happy to submit a pull request.

Browsers Affected

  • Chrome
  • Firefox
  • Edge
  • Safari 9
  • Safari 8
  • IE 11

Versions

  • Polymer: v2.0.0
  • webcomponents: v1.0.1
@web-padawan
Copy link
Contributor

This is not how HTML boolean attributes work. Any value means true, only absense of the attribute means false. If you need another behavior, override it in your own code.

@alim
Copy link
Author

alim commented Jun 16, 2017

@web-padawan - Thank you for the clarification. So you can ignore item #2 in the "Steps to Reproduce" section. I actually do not need different behavior. The Polymer.IronControlState. sets the default value for the disabled property to false. The _deserializeValue(value, type) function receives the value as a "false" string and the type as Boolean. This results in a return value of true which resets the disabled property.

It is quite possible that I am proposing the correction in the wrong piece of code. As I said in the beginning I am very new to using Polymer. I was just trying to be helpful and propose a solution, which seems to make the paper-dropdown-menu behave in the expected manner of being an active element (i.e. not disabled).

@dfreedm
Copy link
Member

dfreedm commented Jun 17, 2017

@alim The paper-dropdown-menu element should already be enabled by default, so I'm confused as to why you would set disabled="false" in the first place, as the demo on webcomponents.org shows: https://www.webcomponents.org/element/PolymerElements/paper-dropdown-menu/demo/demo/index.html

As @web-padawan is suggesting, the existence of the disabled attribute at all counts as true, and simply not using the disabled attribute is the false state. I'm not inclined to add "true" or "false" as values for boolean attributes as the web platform has a bad history with that paradigm, and the existence (or not) of an attribute is simpler.

@alim
Copy link
Author

alim commented Jun 19, 2017

@azakus - Thanks for the reply. I was expecting the paper-dropdown-element to be working be default. You can ignore my misguided attempt to set the disabled="false" attribute. Too many different debugging runs on my side. When we are using Polymer 2.0 with the example code:

<paper-dropdown-menu
  label="Dinosaurs">
    <paper-listbox slot="dropdown-content">
      <paper-item>allosaurus</paper-item>
      <paper-item>brontosaurus</paper-item>
      <paper-item>carcharodontosaurus</paper-item>
    </paper-listbox>
</paper-dropdown-menu>

The element is displayed, but is disabled. We tested this running 1.9 and 2.0 Polymer with in a Rails 5.1 application and got the same results.

I am betting there is a better place to correct this behavior then the one I proposed, I just have not had time to re-run the stack trace. We just noticed that default false value was being passed into the _deserializeValue(value, type) function. When I get some time, I want to put together a stripped down Rails 5.1 app to replicate the behavior. I can then post it to this issue to help trouble shoot it.

@ronak1009
Copy link
Contributor

ronak1009 commented Jun 23, 2017

@alim I got what you are trying to put forward. The actual issue is for deserialization of Boolean values.
in deserializeValue function, it shall return true for all the scenario(s) where the value !== null.
@azakus I propose to update _deserializeValue(value, type) function for Boolean case from:

case Boolean:
            outValue = (value !== null);
            break;

to

case Boolean:
//this shall consider the null/undefined and all other use-cases
            outValue = String(value).toLowerCase() === 'true' ;
            break;

@TimvdLippe
Copy link
Contributor

I am unable to reproduce the described issues: http://jsbin.com/hodizecuyi/edit?html,console,output Since Polymer handles boolean attributes in the same way as the HTML specification does (https://www.w3.org/TR/2008/WD-html5-20080610/semantics.html#boolean) I think this is working as intended.

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

No branches or pull requests

5 participants