Undo Deletion in #31092 #31661
Undo Deletion in #31092 #31661
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| } | ||
|
|
||
| if (currentTheme != null) { | ||
| setSystemChromeSystemUIOverlayStyle(currentTheme); |
There was a problem hiding this comment.
@blasten I added one and I will add some additional tests for the updateSystemUiOverlays method in another PR.
|
|
||
| platformPlugin.setSystemChromeSystemUIOverlayStyle(testStyle); | ||
| platformPlugin.updateSystemUiOverlays(); | ||
| verify(platformPlugin).setSystemChromeSystemUIOverlayStyle(testStyle); |
There was a problem hiding this comment.
could we test for the interactions with the window itself?
Whether setSystemChromeSystemUIOverlayStyle is called or not is not relevant to consumers of these APIs.
There was a problem hiding this comment.
The interaction with the window is already being tested I believe (setSystemChromeSystemUIOverlayStyle just calls setSystemUiOverlayStyle -- example). Is this what you're referring to?
There was a problem hiding this comment.
right similar to that test.
This will ensure that if someone refactors setSystemChromeSystemUIOverlayStyle, this test will break unless the exact same Android APIs are called.
| verify(fakeWindow, never()).setStatusBarContrastEnforced(anyBoolean()); | ||
| verify(fakeWindow, never()).setNavigationBarContrastEnforced(anyBoolean()); | ||
|
|
||
| platformPlugin.setSystemChromeSystemUIOverlayStyle(testStyle); |
There was a problem hiding this comment.
if updateSystemUiOverlays already calls setSystemChromeSystemUIOverlayStyle, then this method could go back to being private. This also decreases the # of calls times(1), which is expected, is that right?
There was a problem hiding this comment.
You're right, I can make it private again, but it will still be called two times because I'm calling to set the style and then to update it
camsim99
added
the
waiting for tree to go green
This reverts commit 1e5b26b.
3 participants
This PR undoes an unnecessary deletion in #31092.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.