Add validation rules for amp-fit-text:1.0 on websites #33464
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey @ampproject/wg-caching! These files were changed: |
krdwan
approved these changes
Mar 25, 2021
alanorozco
approved these changes
Mar 25, 2021
twifkak
approved these changes
Mar 26, 2021
twifkak
reviewed
Mar 30, 2021
Comment on lines
22
to
24
| version: "0.1" | ||
| version: "1.0" | ||
| version: "latest" |
There was a problem hiding this comment.
This removes
requires_usage: EXEMPTED
deprecated_allow_duplicates: true
for the existing cases of 0.1 and latest on AMP (in websites). This could cause errors on existing pages. If we can split up the tag_specs, it should be by version: one tag spec for 1.0 with the new requirements, and one for 0.1 and latest keeping the old leniency.
There was a problem hiding this comment.
Why does this change remove those specifications? I don't see them previously here, are they default values that get modified somehow?
Edit: NVM, I see it wasn't diffed since it's lower down inside the ads block.
There was a problem hiding this comment.
Ping @twifkak, PTAL
|
This PR should not be merged until this thread is resolved. |
caroqliu
changed the title
caroqliu
force-pushed
the
amp-fit-text-validation
branch
from
ae294e0
to
9155974
Compare
caroqliu
changed the title
MichaelRybak
added a commit
to MichaelRybak/amphtml
that referenced
this pull request
Apr 27, 2021
MichaelRybak
added a commit
that referenced
this pull request
Apr 27, 2021
rochapablo
pushed a commit
to rochapablo/amphtml
that referenced
this pull request
Aug 30, 2021
* Allow websites format to import amp-fit-text:1.0 * Add validator test file for 1.0 * 2018 -> 2021 * Add unique spec_name for ad format * Update amp-fit-text validations according to ampproject#33478
rochapablo
pushed a commit
to rochapablo/amphtml
that referenced
this pull request
Aug 30, 2021
* cl/369984001 Revision bump for ampproject#33464 * cl/370113162 Revision bump for ampproject#33611
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note:
bento-fit-textexperiment is still active, so this PR does not "launch" the component, only widens the pool for experimental usage and feedback to include valid AMP documents on the web.Open question:
min-font-size/max-font-sizeattributes withamp-bind. Is it possible to allow[min-font-size]and[max-font-size]foramp-fit-textexclusively on documents where1.0is used? cc / @ampproject/wg-cachingPartial for #28281. cc/ @nainar @dvoytenko