♻️ Define helper for hash params and drop barely-used getMode().log#35628
Merged
rcebulko merged 11 commits intoampproject:mainfrom Aug 12, 2021
Merged
♻️ Define helper for hash params and drop barely-used getMode().log#35628rcebulko merged 11 commits intoampproject:mainfrom
getMode().log#35628rcebulko merged 11 commits intoampproject:mainfrom
Conversation
|
Hey @jridgewell! These files were changed: |
samouri
approved these changes
Aug 12, 2021
jridgewell
requested changes
Aug 12, 2021
src/log.js
Outdated
| defaultLevelWithFunc_(win) { | ||
| // Delegate to the specific resolver. | ||
| return this.levelFunc_(getMode().log, getMode().development); | ||
| return this.levelFunc_(logHashParam(win), getMode().development); |
Contributor
There was a problem hiding this comment.
I believe this is incorrect. Previously we always used the global window (the root AMP doc) and checked it for the hash. The global window is constant (it's per script evaluation), even when we create a log constructor for the embed window. That means the embed iframe doesn't need the #log=foo hash, because only the root window's hash matters.
With this change, we'll be using the embed iframe's window when we create the embed iframe's log. Now the embed iframe needs the #log hash, too.
Suggested change
| return this.levelFunc_(logHashParam(win), getMode().development); | |
| return this.levelFunc_(logHashParam(), getMode().development); |
rcebulko
commented
Aug 12, 2021
| * @private | ||
| */ | ||
| defaultLevelWithFunc_() { | ||
| defaultLevelWithFunc_(opt_win) { |
Contributor
Author
There was a problem hiding this comment.
@jridgewell Updated this so the Window param to defaultLevelWithFunc_ is optional, and is not passed in by defaultLevel_ but the test can still pass it in. I plan to simplify it all further eventually, but not in this PR.
jridgewell
approved these changes
Aug 12, 2021
zoharnyego
added a commit
to apester-dev/amphtml
that referenced
this pull request
Aug 12, 2021
commit b9e41a2 Author: Ryan Cebulko <ryan@cebulko.com> Date: Thu Aug 12 16:47:01 2021 -0400 ♻️ Define helper for hash params and drop barely-used `getMode().log` (ampproject#35628) * Add getHashParams helper * Add win param * Update mode.js to use helper * Add tests * Update callsites * Remove getMode().log uses * Remove log from ModeDef type * Lint fixes * Fix tests * Split up isModeDevelopment commit 0e66e28 Author: Alan Orozco <alanorozco@users.noreply.github.com> Date: Thu Aug 12 13:25:34 2021 -0700 🚮 Remove `<amp-story>` check for `<amp-audio>` actions (ampproject#35643) Removes check from `<amp-audio>` that prevents actions when descending from an `<amp-story>` element. `<amp-story>` now has an action allowlist, so the check was redundant. The action is still prevented, triggering the message '[Action] "AMP-AUDIO.play" is not allowlisted []'. commit 6b46842 Author: honeybadgerdontcare <honeybadgerdontcare@users.noreply.github.com> Date: Thu Aug 12 13:19:24 2021 -0700 Update link to modifications best practices (ampproject#35647) * Update link to modifications best practices * Update dead links commit a59d776 Author: Raghu Simha <rsimha@amp.dev> Date: Thu Aug 12 14:31:55 2021 -0400 🏗 Move Edge testing from GH Actions to CircleCI (ampproject#35630) commit b6476c3 Author: Khoi Doan <khoid@google.com> Date: Thu Aug 12 13:47:16 2021 -0400 ✨ Add bitness to User Agent Client Hint Params in ad requests (ampproject#35612) * Add User Agent Client Hint Params to Google Ads. * Update the param names. * Add timeout for UACH promise (1 second). * Use fake timers for timeout test. * Lint * Add exception for dependency on promise.js. * Add bitness field to User Agent Client Hints params * Fix merge * Fix merge again * Lint commit 87d5d14 Author: Ryan Cebulko <ryan@cebulko.com> Date: Thu Aug 12 13:43:51 2021 -0400 ♻️ Remove `version` from legacy mode object (ampproject#35624) * Remove `version` from legacy mode object * Restore version to mode-object for now * Remove version entirely * Lint fixes * Fix transformer test fixtures * Fix test commit 00b8e7e Author: Brandon Suen <brandonbsuen@google.com> Date: Thu Aug 12 10:31:56 2021 -0700 📖 [Story interactive] Add Documentation for Image Quizzes and Polls (ampproject#35618) * Documentation text added * Add images and source code * Reorganize in alphabetical order * Reordering * Add additional attributes * Fix typo * Fix typo * Fix typo * Update screenshots * Add note about image sizes * Revise note about image size commit 9813cea Author: Ryan Cebulko <ryan@cebulko.com> Date: Thu Aug 12 13:27:55 2021 -0400 ♻️ #core form, modal, amp-element-helpers ("real" diff in desc) (ampproject#35608) * Update src/form code * Update src/modal code * Update src/video-interface code * Move src/{form,modal,amp-element-helpers} -> #core/dom * Fix types * update dep-check config * Remove bad assert * Update imports of form,modal,amp-element-helpers * Fix rebase error commit 4ed4b92 Author: Philip Bell <philip.hunter.bell@gmail.com> Date: Thu Aug 12 12:38:05 2021 -0400 Null check on orientation lock. (ampproject#35639) commit 5ff32ad Author: Chris Antaki <ChrisAntaki@gmail.com> Date: Wed Aug 11 15:12:58 2021 -0700 SwG Release 0.1.22.179 (ampproject#35629) commit 2106624 Author: Matias Szylkowski <mszylkowski@google.com> Date: Wed Aug 11 17:13:54 2021 -0400 📖 [Story sidebar] Removed sidebar from docs (ampproject#35616) * removed sidebar and bookend docs * removed unused import * revert test removals * Add newline on visual test html commit d21ecc0 Author: Ryan Cebulko <ryan@cebulko.com> Date: Wed Aug 11 17:01:02 2021 -0400 ♻️ #core: error helpers (ampproject#35621) * Define core/error helpers * Drop createErrorVargs * Restore name assignment in Log#error * Typechecking/extern * Remove unused error_ helper * Typo * Add @ returns commit 6404407 Author: Brandon Suen <brandonbsuen@google.com> Date: Wed Aug 11 13:33:02 2021 -0700 ✅ [Story interactive] Rename Example Image Quiz and Poll Stories to Use .amp.html Extension (ampproject#35511) * Rename examples to use .amp.html extension * Remove confetti from results
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
It's only used in
log.js, so we move the parsing etc. there.