hello reviewers, thanks in advance for your time, this is the frontend part for
https://chromium-review.googlesource.com/c/chromium/src/+/7580113
see bug: https://issues.chromium.org/issues/40807290#comment19 for a video of the feature.
please let me know if you want me to address anything!
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thanks for the CL, looks good overall! Would be great if you could cover the device toolbar updates with tests.
// When unlocked, restore the button to its normal state.Could you add tests covering this?
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
// When unlocked, restore the button to its normal state.Could you add tests covering this?
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
beforeEach(() => {Can you use stubNoopSettings?
const buttons = toolbar.element().querySelectorAll('devtools-button.toolbar-button');querySelectorAll<Buttons.Button.Button> accepts a type argument, means you don't need to cast below.
throw new Error('Could not find rotate button');Assert on the result instead?
emulationModel!.screenOrientationLockChanged({Superfluous non-null assertion, also below.
No need to include these changes in this CL, they get autorolled from chromium. Once that happens, you can rebase the CL to get rid of them.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
target.registerEmulationDispatcher(this);nit: let's move to the end of the constructor to be safer in case registerEmulationDispatcher dispatches events?
this.#screenOrientationLocked = false;I think we might need to dispatched the Updated event here?
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
nit: let's move to the end of the constructor to be safer in case registerEmulationDispatcher dispatches events?
Done
I think we might need to dispatched the Updated event here?
Done. Added `this.dispatchEventToListeners(Events.UPDATED)` after resetting `#screenOrientationLocked`, matching the pattern in `onScreenOrientationLockChanged`.
beforeEach(() => {Helmut JanuschkaCan you use stubNoopSettings?
Done
const buttons = toolbar.element().querySelectorAll('devtools-button.toolbar-button');querySelectorAll<Buttons.Button.Button> accepts a type argument, means you don't need to cast below.
Done
Assert on the result instead?
Done
Superfluous non-null assertion, also below.
Done
No need to include these changes in this CL, they get autorolled from chromium. Once that happens, you can rebase the CL to get rid of them.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
it appears there are some test failures?
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
Still LGTM! Any ideas about the tests?
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
Still LGTM! Any ideas about the tests?
Done
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
DevTools: Support screen.orientation.lock() in device emulation
Handle the new CDP event Emulation.screenOrientationLockChanged to rotate
the emulated device to match the locked orientation and disable the rotate
button while the lock is active.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |