[web] Remove unused function parameter #41700
Conversation
| @@ -9,8 +9,6 @@ import 'text_editing/text_editing.dart'; | |||
| // Applies the required global CSS to an incoming [DomCSSStyleSheet] `sheet`. | |||
| void applyGlobalCssRulesToSheet( | |||
| DomHTMLStyleElement styleElement, { | |||
| required bool hasAutofillOverlay, | |||
| String cssSelectorPrefix = '', | |||
There was a problem hiding this comment.
Well, turns out the cssSelectorPrefix param is used by the new text editing host node:
I have 2 questions:
- Is that necessary because it's outside of the shadow dom?
- Since the styles are scoped to
flutter-view, should we attach it somewhere global so that it applies to allflutter-views?
There was a problem hiding this comment.
Isn't it more necessary now that it's out of the shadowDOM? I thought one of the big gains we got from the shadowDOM was that the styles don't leak and affect things outside of the app's context. Now that inputs are outside of the shadowDOM, the selector prefix makes sure we're only applying these styles to flutter app inputs.
Option 2 sounds like a good idea. Also having multiple flutter-views means multiple <style> elements get created unnecessarily currently. Having one style sheet at the top level would probably be a cleaner approach.
There was a problem hiding this comment.
Isn't it more necessary now that it's out of the shadowDOM? I thought one of the big gains we got from the shadowDOM was that the styles don't leak and affect things outside of the app's context. Now that inputs are outside of the shadowDOM, the selector prefix makes sure we're only applying these styles to flutter app inputs.
Alright, cool, that's what I thought too. So you are saying "yes" to my first question :)
Option 2 sounds like a good idea. Also having multiple
flutter-views means multiple<style>elements get created unnecessarily currently. Having one style sheet at the top level would probably be a cleaner approach.
Yep. I'll leave it as is for now. I'm happy to revisit and do this when we start figuring things out for multi-view.
There was a problem hiding this comment.
Oh why did I read your comment as saying it was "unnecessary" 😂
Yeah we're in agreement. Sounds good to me on revisiting for later!
There was a problem hiding this comment.
Done. I put the cssSelectorPrefix parameter back. Now the PR is only removing the hasAutofillOverlay parameter.
…126018) flutter/engine@7d70dc8...a0906bd 2023-05-03 bdero@google.com [Impeller] Load instead of restore drawing for non-MSAA passes (flutter/engine#41676) 2023-05-03 jonahwilliams@google.com Remove physical model layer (flutter/engine#41593) 2023-05-03 gspencergoog@users.noreply.github.com Reland: "Determine lifecycle by looking at window focus also" (#41094) (flutter/engine#41702) 2023-05-03 skia-flutter-autoroll@skia.org Roll Skia from 887e58fff02f to 93c869da03f5 (12 revisions) (flutter/engine#41705) 2023-05-03 mdebbar@google.com [web] Improve null safety for color->css (flutter/engine#41699) 2023-05-03 mdebbar@google.com [web] Remove unused function parameter (flutter/engine#41700) 2023-05-03 godofredoc@google.com Migrate Android Engine Debug to Engine V2. (flutter/engine#41687) 2023-05-03 kjlubick@users.noreply.github.com Migrate SkSurface::getBackend* methods to static functions (flutter/engine#41693) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jimgraham@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
The
hasAutofillOverlayandcssSelectorPrefixparameters aren't really used in any useful way. Let's remove them.