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

Revert converting decimal image sizes to integers for sizer SVG generation #4863

Closed
schlessera opened this issue Jun 16, 2020 · 1 comment · Fixed by #4864
Closed

Revert converting decimal image sizes to integers for sizer SVG generation #4863

schlessera opened this issue Jun 16, 2020 · 1 comment · Fixed by #4864
Labels
Optimizer WS:Perf Work stream for Metrics, Performance and Optimizer
Milestone

Comments

@schlessera
Copy link
Collaborator

Feature description

For the issue with decimal image sizes described at #4493, we resorted to a workaround while the validator was being adapted, by truncating the image sizes to integer values for the SVG generation.

As ampproject/amphtml#27528 has been fixed upstream, we should revert this work-around.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Decimal sizes are used as-is for sizer generation.

Implementation brief

QA testing instructions

Demo

Changelog entry

@schlessera schlessera added this to the v1.5.4 milestone Jun 16, 2020
@schlessera schlessera self-assigned this Jun 16, 2020
@kmyram kmyram assigned johnwatkins0 and unassigned schlessera Jun 16, 2020
@johnwatkins0
Copy link
Contributor

Moving this to QA Passed now, as I was able to add an AMP image with a decimal dimension locally, and no validation errors appeared.

Screen Shot 2020-06-16 at 4 55 15 PM

Screen Shot 2020-06-16 at 4 55 35 PM

@kmyram kmyram added the WS:Perf Work stream for Metrics, Performance and Optimizer label Aug 5, 2020
@johnwatkins0 johnwatkins0 removed their assignment Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Optimizer WS:Perf Work stream for Metrics, Performance and Optimizer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants