[web] Apply autocapitalize to text editing elements#188351
Conversation
2eaa8af to
57c09ce
Compare
There was a problem hiding this comment.
Code Review
This pull request refactors how onscreen canvases are styled as overlays by dynamically setting the position: absolute CSS property only when platform views are present, rather than hardcoding it. It also centralizes the application of the autocapitalize attribute in text editing strategies and updates corresponding tests. The review feedback suggests wrapping a test's JS configuration override in a try-finally block to prevent test pollution and correcting a misleading copy-pasted comment in the text editing tests.
I am having trouble creating individual review comments. Click here to see my feedback.
engine/src/flutter/lib/web_ui/test/ui/platform_view_position_test.dart (25-64)
To prevent test pollution, wrap the test body in a try-finally block to ensure that engine.debugOverrideJsConfiguration(null) is always executed, even if the test fails or throws an exception. This ensures subsequent tests are not affected by a leftover global JS configuration override.
test('base onscreen canvas is not positioned as an overlay', () async {
if (!isCanvasKit) {
return;
}
// Force multi-surface mode to ensure OnscreenCanvasProvider is used.
engine.debugOverrideJsConfiguration(
<String, Object?>{'canvasKitForceMultiSurfaceRasterizer': true}.jsify()
as engine.JsFlutterConfiguration?,
);
try {
// Reset the renderer to ensure it is created with the new configuration.
engine.renderer.debugResetRasterizer();
engine.renderer.debugClear();
// Create a scene with only Flutter content. This uses the base canvas, not
// an overlay canvas interleaved with platform views.
final recorder = ui.PictureRecorder();
final canvas = ui.Canvas(recorder);
canvas.drawRect(
const ui.Rect.fromLTWH(0, 0, 100, 100),
ui.Paint()..color = const ui.Color(0xFFFF0000),
);
final sb = ui.SceneBuilder();
sb.addPicture(ui.Offset.zero, recorder.endRecording());
await renderScene(sb.build());
final DomElement canvasElement = (implicitView as engine.EngineFlutterView).dom.sceneHost
.querySelectorAll('canvas')
.single;
expect(
canvasElement.style.position,
isNot('absolute'),
reason: 'Base canvas should not be styled as a platform-view overlay.',
);
} finally {
engine.debugOverrideJsConfiguration(null);
}
});References
- Code should be tested and follow the guidance described in the writing effective tests guide. (link)
engine/src/flutter/lib/web_ui/test/engine/text_editing_test.dart (1931)
The comment mentions creating a configuration with an AutofillGroup of four text fields, but the code actually creates a single text field configuration with sentences capitalization. This appears to be a copy-paste comment error and should be corrected to avoid confusion.
// Create a configuration with sentences capitalization.
References
- Optimize for readability: Code is read more often than it is written. (link)
57c09ce to
c14e255
Compare
c14e255 to
53240a1
Compare
|
autosubmit label was removed for flutter/flutter/188351, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
Fixes #187231
Problem
Flutter web does not apply
TextCapitalizationto the hidden.flt-text-editinginput for all text editing strategies. On Chromebook OSK, this can cause the first typed letter to be capitalized even when the app usesTextCapitalization.none.Fix
Apply
autocapitalizein the sharedDefaultTextEditingStrategy.applyConfigurationpath so all web strategies write the correct DOM attribute.Demo
Steps: enable Chromebook on-screen keyboard, focus the Flutter text field, dismiss the keyboard, refocus the field, then type
a.Before: the hidden Flutter input is missing
autocapitalize, and Chromebook OSK may typeA.After: the hidden Flutter input has
autocapitalize="off", and Chromebook OSK typesa.Manual validation
autocapitalizeon.flt-text-editing.autocapitalize="off"on.flt-text-editing.