[animation-trigger] Rename range boundaries of TimelineTrigger IDLs [chromium/src : main]
AI Code Reviewer (Gerrit)
AI Code Reviewer added 1 comment![]( "Open in Gerrit")
Boundary* entry_exit,nit: Variable 'entry_exit' is unclear; consider a more descriptive name like 'entry_end' to match 'active_end' and the member variable 'entry_range_end_'. (Blink Style Guide: Naming)
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Awogbemila (Gerrit)
David Awogbemila added 1 comment![]( "Open in Gerrit")
nit: Variable 'entry_exit' is unclear; consider a more descriptive name like 'entry_end' to match 'active_end' and the member variable 'entry_range_end_'. (Blink Style Guide: Naming)
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Done
Related details
- Robert Flack
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Robert Flack (Gerrit)
Robert Flack added 1 comment![]( "Open in Gerrit")
entryRangeEnd: `${TRIGGER_END_PX}px`Does it work if you just set activeRange? Can you make the test do this?
Per the discussion the intention is that developers will just set the active range unless they specifically want to narrow the entry range. See https://github.com/w3c/csswg-drafts/issues/13174#:~:text=primary%20thing%20author%20wants%20to%20set%20is%20active%20range%2C%20don%27t%20want%20to%20set%20an%20entry%20rannge%2C%20then%20entry%20range%20is%20a%20narrowing%20of%20the%20primary%20thing%20they%27re%20setting
primary thing author wants to set is active range, don't want to set an entry rannge, then entry range is a narrowing of the primary thing they're setting
Related details
- David Awogbemila
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Awogbemila (Gerrit)
David Awogbemila added 1 comment![]( "Open in Gerrit")
// Both are undefined: normal auto.I suppose another option is to leave "auto" in the realm of CSS. I.e. "auto" would only be valid as a CSS value as a way of setting the value of the underlying object `TimelineTrigger` object whose only valid values are the same as `Animation.rangeStart`.
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Awogbemila (Gerrit)
David Awogbemila added 1 comment![]( "Open in Gerrit")
entryRangeEnd: `${TRIGGER_END_PX}px`Does it work if you just set activeRange? Can you make the test do this?
Per the discussion the intention is that developers will just set the active range unless they specifically want to narrow the entry range. See https://github.com/w3c/csswg-drafts/issues/13174#:~:text=primary%20thing%20author%20wants%20to%20set%20is%20active%20range%2C%20don%27t%20want%20to%20set%20an%20entry%20rannge%2C%20then%20entry%20range%20is%20a%20narrowing%20of%20the%20primary%20thing%20they%27re%20setting
primary thing author wants to set is active range, don't want to set an entry rannge, then entry range is a narrowing of the primary thing they're setting
- Robert Flack
Robert Flack (Gerrit)
Robert Flack voted and added 2 comments![]( "Open in Gerrit")
Votes added by Robert Flack
| Code-Review | +1 |
2 comments
use_active_range=false) {Personally I think it's better to just ensure you have at least one explicit test that directly tests each type of trigger rather than trying to test every combination of options with parameterization. I.e. given that setting one range implicitly sets the other, other than ensuring that this happens all of the extra assertions are redundant.
entryRangeEnd: `${TRIGGER_END_PX}px`David AwogbemilaDoes it work if you just set activeRange? Can you make the test do this?
Per the discussion the intention is that developers will just set the active range unless they specifically want to narrow the entry range. See https://github.com/w3c/csswg-drafts/issues/13174#:~:text=primary%20thing%20author%20wants%20to%20set%20is%20active%20range%2C%20don%27t%20want%20to%20set%20an%20entry%20rannge%2C%20then%20entry%20range%20is%20a%20narrowing%20of%20the%20primary%20thing%20they%27re%20setting
primary thing author wants to set is active range, don't want to set an entry rannge, then entry range is a narrowing of the primary thing they're setting
Added a test case for this now.
- David Awogbemila
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Awogbemila (Gerrit)
David Awogbemila voted and added 1 comment![]( "Open in Gerrit")
Votes added by David Awogbemila
| Commit-Queue | +2 |
1 comment
Personally I think it's better to just ensure you have at least one explicit test that directly tests each type of trigger rather than trying to test every combination of options with parameterization. I.e. given that setting one range implicitly sets the other, other than ensuring that this happens all of the extra assertions are redundant.
Okay, I think that makes the tests added to `TimelineTrigger/constructor.html` sufficient. I've reverted this change.
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Awogbemila (Gerrit)
David Awogbemila voted![]( "Open in Gerrit")
| Code-Review | +1 |
| Commit-Queue | +2 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/57156.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[animation-trigger] Rename range boundaries of TimelineTrigger IDLs
This is the last part of the rename resolved[1] on.
[1] https://github.com/w3c/csswg-drafts/issues/13174
- M android_webview/test/data/web_tests/webexposed/global-interface-listing-expected.txt
- M third_party/blink/renderer/bindings/generated_in_core.gni
- M third_party/blink/renderer/core/animation/animation_test.cc
- M third_party/blink/renderer/core/animation/animation_trigger_test.cc
- M third_party/blink/renderer/core/animation/css/css_animations.cc
- M third_party/blink/renderer/core/animation/css/css_animations_test.cc
- M third_party/blink/renderer/core/animation/timeline_trigger.h
- M third_party/blink/renderer/core/animation/timeline_trigger.idl
- M third_party/blink/renderer/core/animation/timeline_trigger_options.idl
- M third_party/blink/renderer/core/animation/timeline_trigger_range.cc
- M third_party/blink/renderer/core/animation/timeline_trigger_range.h
- M third_party/blink/web_tests/external/wpt/scroll-animations/animation-trigger/animation-trigger-addAnimation.tentative.html
- M third_party/blink/web_tests/external/wpt/scroll-animations/animation-trigger/animation-trigger-disarmed-by-apis.tentative.html
- M third_party/blink/web_tests/external/wpt/scroll-animations/animation-trigger/animation-trigger-multiple-animations.tentative.html
- M third_party/blink/web_tests/external/wpt/scroll-animations/animation-trigger/animation-trigger-multiple-triggers.tentative.html
- M third_party/blink/web_tests/external/wpt/scroll-animations/animation-trigger/animation-trigger-range-px-roundtrip.tentative.html
- M third_party/blink/web_tests/external/wpt/scroll-animations/animation-trigger/support/support.js
- M third_party/blink/web_tests/external/wpt/web-animations/interfaces/TimelineTrigger/constructor.html
- M third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt
Code-Review: +1 by David Awogbemila, +1 by Robert Flack
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/57156
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |