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

amp-bind: Spurious verify warnings for [height] #12028

Closed
scott-rothman opened this issue Nov 13, 2017 · 4 comments
Closed

amp-bind: Spurious verify warnings for [height] #12028

scott-rothman opened this issue Nov 13, 2017 · 4 comments

Comments

@scott-rothman
Copy link

scott-rothman commented Nov 13, 2017

What's the issue?

Development mode throws the following error amp-bind: Default value for [height] does not match first expression result (550). This can result in unexpected behavior after the next state change. even though the default value is in fact the same as the first expression result

How do we reproduce the issue?

  1. Set an amp-carousel to have a dynamic height via [height]
  2. Define the height to the appropriate default value
  3. Load the page in development mode and observe the console

My specific example:

<amp-carousel
   [height]="visible ? 720 : 550"
   height="550"
   width="400"
   layout="responsive"
   type="slides">
    <div class="carousel__slide">
       <p>Text content here</p>
       <button [text]="visible ? 'See Less' : 'See More'" 
           on="tap:AMP.setState({visible: !visible})">See More</button>
       <ul [class]="visible ? 'show' : 'hide'" class="hide">
          <li>Lorem ipsum dolor sit amet, consectetur adipisicing elit.</li>
          <li>Lorem ipsum dolor sit amet, consectetur adipisicing elit.</li>
          <li>Lorem ipsum dolor sit amet, consectetur adipisicing elit.</li>
       </ul>
    </div>
</amp-carousel>

What browsers are affected?

Seemingly all browsers

Which AMP version is affected?

Version 1509747505247

@scott-rothman
Copy link
Author

Addendum: I discussed this in the amphtml slack channel and was requested to file an issue

@ampprojectbot ampprojectbot added this to the Pending Triage milestone Nov 13, 2017
@aghassemi
Copy link
Contributor

/to @choumx Issues seems to be with verifyBinding_ which other than text and class seems to assume everything else is a boolean attribute. The bug does not impact actual behaviour, just wrong logging in development mode.

@dreamofabear
Copy link

Thanks for filing this issue. This message is actually just a warning, though it has been confusing for many others too. We'll fix this and clarify the messaging.

@dreamofabear dreamofabear changed the title Issue with height validation in development mode amp-bind: Spurious verify warnings for [height] Nov 15, 2017
@dreamofabear
Copy link

Fixed by #13236.

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

4 participants