This PR removes unnecessary local variables and related comments in `Editable` and `RenderParagraph` as both now use another `TextPainter` instance for intrinsics cache.
Test-exempt: minor refactor and comments.
Here's another PR with a couple of typos fixed. As you can see there was a typo in _fileReferenceI**n**dentifiers_, in class _ParsedProjectInfo._ Maybe we should do some check on that since I'm not sure if that property is used somewhere outside Flutter?
`RendererBinding.renderView` is deprecated. The doc should link to `RendererBinding.renderViews`, which also matches the context since the sentence is talking about multiple `RenderView`s and not just the legacy singleton.
This pull request aims for improved readability, based on issue #146600.
```dart
// before
Set<Color> _distinctVisibleColors() {
final Set<Color> distinctVisibleColors = <Color>{};
if (top.style != BorderStyle.none) {
distinctVisibleColors.add(top.color);
}
if (right.style != BorderStyle.none) {
distinctVisibleColors.add(right.color);
}
if (bottom.style != BorderStyle.none) {
distinctVisibleColors.add(bottom.color);
}
if (left.style != BorderStyle.none) {
distinctVisibleColors.add(left.color);
}
return distinctVisibleColors;
}
// after
Set<Color> _distinctVisibleColors() {
return <Color>{
if (top.style != BorderStyle.none) top.color,
if (right.style != BorderStyle.none) right.color,
if (bottom.style != BorderStyle.none) bottom.color,
if (left.style != BorderStyle.none) left.color,
};
}
```
Most of the repo should be covered in this PR (aside from `flutter_tools/`, since there was a lot going on in there).
This cleans up a few sliver classes, like moving RenderSliverVariedExtentList to the rendering layer (it was in the widgets layer), and moving SliverVariedExtentList to live with its sibling subclasses, SliverFixedExtentList, SliverList, and so on.
I moved these while working on SliverTree, so figure I should break out into a separate change.
SliverTree and SliverCarousel (both inbound in separate changes) will also be subclasses of RenderSliverFixedExtentBoxAdaptor, organizing them together felt easier to work with.
Related to https://github.com/flutter/flutter/issues/114299 and https://github.com/flutter/flutter/issues/125980
Fixes bugs in the text selection positioning calculations so that they work even when the field is scaled. In many cases, we were simply translating things around without applying the proper localToGlobal (or vice versa) transform.
| Before | After |
| --- | --- |
| <img src="https://github.com/flutter/flutter/assets/389558/a5a45472-98c5-4cdf-b382-218971fd9404" /> | <img src="https://github.com/flutter/flutter/assets/389558/f396a1af-2546-4e38-a9d9-6c6edfa38d94" /> |
Partial fix for: https://github.com/flutter/flutter/issues/144685
It looks like there are other problems where transforms aren't applied properly. Putting a transform at the root of the application, above MaterialApp, will expose more problems.
<details>
<summary>Sample code</summary>
```dart
import 'package:flutter/material.dart';
import 'package:webview_flutter/webview_flutter.dart';
void main() => runApp(const _App());
class _App extends StatelessWidget {
const _App();
@override
Widget build(BuildContext context) {
return const MaterialApp(home: _Home());
}
}
class _Home extends StatefulWidget {
const _Home();
@override
State<_Home> createState() => _HomeState();
}
class _HomeState extends State<_Home> {
final _controller = WebViewController();
final TextEditingController textEditingController = TextEditingController(
text: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.',
);
final OverlayPortalController overlayPortalController = OverlayPortalController();
@override
void initState() {
super.initState();
_controller
..setJavaScriptMode(JavaScriptMode.unrestricted)
..loadRequest(Uri.https('api.flutter.dev'));
}
@override
Widget build(BuildContext context) {
overlayPortalController.show();
return Scaffold(
appBar: AppBar(
title: const Text('Scaled WebView Tester'),
),
body: Stack(
children: <Widget>[
Transform.scale(
alignment: Alignment.topLeft,
scale: 0.5,
child: TextField(
controller: textEditingController,
maxLines: null,
),
),
OverlayPortal(
controller: overlayPortalController,
overlayChildBuilder: (BuildContext context) {
return Positioned(
top: 0.0,
left: 0.0,
child: SizedBox(
height: 1000,
width: 1000,
child: Stack(
children: <Widget>[
Positioned(
top: 90.0,
left: 0.0,
child: Container(
height: 1.0,
width: 1000,
color: Colors.blue,
),
),
Positioned(
top: 102.0,
left: 0.0,
child: Container(
height: 1.0,
width: 1000,
color: Colors.blue,
),
),
],
),
),
);
},
),
],
),
);
}
}
```
</details>
This change adds support for triple click to select a paragraph at the clicked position and triple click + drag to extend the selection paragraph-by-paragraph when using the SelectionArea widget.
This PR also:
* Makes `Text` widgets a `SelectionContainer` if a parent `SelectionRegistrar` exists.
* Fixes issues with selectable ordering involving `WidgetSpan`s.
Fixes: https://github.com/flutter/flutter/issues/104552
`@visibleForOverriding` + `@protected` unfortunately does not catch the case where a `compute*` method was overridden in a subtype and the overide was called in that same type's implementation.
I did not add a `flutter_ignore` for this because it doesn't seem there will be false positives.
Use a dedicated `TextPainter` for intrinsic size calculation in `RenderEditable` and `RenderParagraph`.
This is an implementation detail so the change should be covered by existing tests. Performance wise this shouldn't be significantly slower since SkParagraph [caches the result of slower operations across different paragraphs](9c62e7b382/modules/skparagraph/src/ParagraphCache.cpp (L254-L272)). Existing benchmarks should be able to catch potential regressions (??).
The reason for making this change is to make sure that intrinsic size computations don't destroy text layout artifacts, so I can expose the text layout as a stream of immutable `TextLayout` objects, to signify other render objects that text-layout-dependent-cache (such as caches for `getBoxesForRange` which can be relatively slow to compute) should be invalidated and `markNeedsPaint` needs to be called if the painting logic depended on text layout.
Without this change, the intrinsics/dry layout calculations will add additional events to the text layout stream, which violates the "dry"/non-destructive contract.
When a Sliver with items is outside of the Viewport, but within the Viewport's `cacheExtent`, the framework should create SemanticNodes for the items even though they are out of view. However, for this to work, the Sliver's geometry must have a `cacheExtent` (how much space the sliver took up of the Viewport's `cacheExtent`) greater than 0, otherwise it is [excluded](f01ce9f4cb/packages/flutter/lib/src/rendering/viewport.dart (L311-L315)).
`SliverFillRemaining` widgets that fall outside the viewport did not have this set and therefore were being excluded when SemanticNodes were created, even if they were within the Viewport's `cacheExtent`. This PR sets the `cacheExtent` for `SliverFillRemaining` widgets.
In addition, `RenderSliverFillRemainingWithScrollable` would get dropped from the semantic tree because it's child had a size of 0 when outside the remaining paint extent. To fix, we give the child a `maxExtent` of the sliver's `cacheExtent` if it's outside the remaining paint extent but within the viewport's cacheExtent.
Fixes https://github.com/flutter/flutter/issues/142065.
Definitions:
* `RenderViewport.cacheExtent`:
```dart
/// The viewport has an area before and after the visible area to cache items
/// that are about to become visible when the user scrolls.
///
/// Items that fall in this cache area are laid out even though they are not
/// (yet) visible on screen. The [cacheExtent] describes how many pixels
/// the cache area extends before the leading edge and after the trailing edge
/// of the viewport.
///
/// The total extent, which the viewport will try to cover with children, is
/// [cacheExtent] before the leading edge + extent of the main axis +
/// [cacheExtent] after the trailing edge.
///
/// The cache area is also used to implement implicit accessibility scrolling
/// on iOS: When the accessibility focus moves from an item in the visible
/// viewport to an invisible item in the cache area, the framework will bring
/// that item into view with an (implicit) scroll action.
```
* `SliverGeometry.cacheExtent`:
```dart
/// How many pixels the sliver has consumed in the
/// [SliverConstraints.remainingCacheExtent].
```
* `SliverContraints.remainingCacheExtent`:
```dart
/// Describes how much content the sliver should provide starting from the
/// [cacheOrigin].
///
/// Not all content in the [remainingCacheExtent] will be visible as some
/// of it might fall into the cache area of the viewport.
///
/// Each sliver should start laying out content at the [cacheOrigin] and
/// try to provide as much content as the [remainingCacheExtent] allows.
```
I was doing some debugging on a RenderSliver subclass, and found
that SliverConstraints.toString was missing the precedingScrollExtent
field.
Add that, and add both that field and userScrollDirection to the
`==` and hashCode implementations, which had been skipping them,
so that all three methods now handle all the class's fields.
Originally, my aim was just to refactor (as per usual), but while messing around with the `TableBorder.symmetric` constructor, I realized that `borderRadius` was missing!
This pull request makes a few class constructors more efficient, and it fixes#144277 by adding the missing parameter.
<br>
This pull request is part of the effort to solve issue #136139.
The previous [`switch` expressions PR](https://github.com/flutter/flutter/pull/143496) was comprised of many simple changes throughout `flutter/lib/src/`, but due to some more in-depth refactoring in `flutter/lib/src/rendering/`, I decided to submit the changes to this directory as a separate pull request.
There was really just one function that I changed significantly; I'll add a comment for explanation.
This pull request fixes#143803 by taking advantage of Dart's null-aware operators.
And unlike `switch` expressions ([9 PRs](https://github.com/flutter/flutter/pull/143634) and counting), the Flutter codebase is already fantastic when it comes to null-aware coding. After refactoring the entire repo, all the changes involving `?.` and `??` can fit into a single pull request.
My RenderSliverMultiBoxAdaptor/RenderSliverFixedExtentList/RenderSliverVariedExtentList yak shave continues.
I've been subclassing RenderSliverVariedExtentList for SliverTree and have found some opportunities for clean up.
There is a larger clean up I'd like to do, but this week SliverTree comes first.
I noticed these methods were getting repeated ð, and I was about to repeat them again ð for the tree, so I figured bumping them up to the base class was better than continuing to copy-paste the same methods.
Multiple methods in `RenderSliverFixedExtentBoxAdaptor` pass a `double itemExtent` for computing things like what children will be laid out, what the max scroll offset will be, and how the children will be laid out.
Since `RenderSliverFixedExtentBoxAdaptor` was further subclassed to support a `itemExtentBuider` in `RenderSliverVariedExtentList`, these itemExtent parameters became useless when using that RenderObject. Reading through `RenderSliverFixedExtentBoxAdaptor.performLayout`, the remaining artifacts of passing around itemExtent make it hard to follow when it is irrelevant.
`RenderSliverFixedExtentBoxAdaptor.itemExtent` is available from these methods, so it does not need to pass it. It is redundant API.
Plus, if a bogus itemExtent is passed for some reason, errors will ensue and the layout will be incorrect. ð£ ð¥
Deprecating so we can remove these for a cleaner API. Unfortunately this is not supported by dart fix, but the fact that these methods are protected means usage outside of the framework is likely minimal.
Fixes https://github.com/flutter/flutter/issues/138912
Change `ItemExtentBuilder`'s return value nullable, it should return null if asked to build an item extent with a greater index than exists.
Fixes https://github.com/flutter/flutter/issues/79495
This is basically a reland of https://github.com/flutter/flutter/pull/79607.
Currently when the cursor is invalid, arrow key navigation / typing / backspacing doesn't work since the cursor position is unknown.
Showing the cursor when the selection is invalid gives the user the wrong information about the current insert point in the text.
This is going to break internal golden tests.
This PR is the 8áµÊ° step in the journey to solve issue #136139 and make the entire Flutter repo more readable.
(previous pull requests: #139048, #139882, #141591, #142279, #142634, #142793, #143293)
I did a pass through all of `packages/flutter/lib/src/` and found a whole bunch of `switch` statements to improve: most of them were really simple, but many involved some thorough refactoring.
This pull request is just the complicated stuff. ð I'll make comments to describe the changes, and then in the future there will be another PR (and it'll be much easier to review than this one).
*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*
*List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.*
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
Fixes https://github.com/flutter/flutter/issues/138610.
When `RenderImage` receives a new `Image` it only needs to fire up the layout machinery when the dimensions of the image have changed compared to the previous image. If the dimensions are the same, a repaint is sufficient.
Fixes https://github.com/flutter/flutter/issues/142183
This fixes a bug in the SliverGeometry of SliverMainAxisGroup. The cacheExtent represents how many pixels the sliver has consumed in the SliverConstraints.remainingCacheExtent. Since it was not set, slivers that came after a SliverMainAxisGroup that filled the whole screen did not properly lay out their own children, in some cases making lazy sliver more eager than they should be.
I continued [my mission](https://github.com/flutter/flutter/pull/141431) to find as many typos as I could. This time it's a smaller set than before.
There is no need for issues since it's a typo fix.
