Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@GaryQian
Copy link
Contributor

This exposes the RectHeightStyle and RectWidthStyle engine API as BoxHeightStyle and BoxWidthStyle enums in dart:ui.

The styles are presented as optional parameters, with both defaulting to "tight" which is what we currently do.

The new options include max, includeLineSpacingMiddle, includeLineSpacingTop, and includeLineSpacingBottom. What they do are documented in the code.

@GaryQian
Copy link
Contributor Author

cc @Hixie For API review

lib/ui/text.dart Outdated
enum BoxHeightStyle {
/// Provide tight bounding boxes that fit heights per run. This style may result
/// in uneven bounding boxes that do not nicely connect with the boxes above,
/// below, and adjacent.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

above and below are both adjacent.
Maybe just "adjacent boxes"?

lib/ui/text.dart Outdated
///
/// The top and bottom of each box will cover half of the
/// space above and half of the space below the line. The text should be
/// centered vertically within the box.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might not actually be vertically centered, consider a line with a high line height containing a text span with a relatively small font size. the text will appear to be vertically near the bottom.

lib/ui/text.dart Outdated
/// The top edge of each line should be the same as the bottom edge
/// of the line above. There should be no gaps in vertical coverage given any
/// amount of line spacing. Line spacing is not included above the first line
/// below the last line due to no additional space present there.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing "and" at the start of line 979?

due to -> as there is

@chinmaygarde
Copy link
Member

Is this good to go?

@GaryQian
Copy link
Contributor Author

For reference, this exposes the changes made in #6591

lib/ui/text.dart Outdated
/// Adds up to two additional boxes as needed at the beginning and/or end
/// of each line so that the widths of the boxes in line are the same width
/// as the widest line in the paragraph. The additional boxes are only added
/// when the first/last box does not span the max width of the paragraph.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The additional boxes on each line are only added when the relevant box at the relevant edge of that line does not span the maximum width of the paragraph."

lib/ui/text.dart Outdated
/// Returns a list of text boxes that enclose the given text range.
List<TextBox> getBoxesForRange(int start, int end) native 'Paragraph_getRectsForRange';
///
/// [boxHeightStyle] and [boxWidthStyle] allow customization of how the boxes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid starting a sentence with a lowercase letter. To avoid this, use something like "The [foo] argument..."

lib/ui/text.dart Outdated
///
/// [boxHeightStyle] and [boxWidthStyle] allow customization of how the boxes
/// are bound vertically and horizontally. Both style parameters default to
/// the tight option, which will provide close fitting boxes and will not
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

close-fitting

///
/// See [BoxHeightStyle] and [BoxWidthStyle] for full descriptions of each option.
List<TextBox> getBoxesForRange(int start, int end, {BoxHeightStyle boxHeightStyle = BoxHeightStyle.tight, BoxWidthStyle boxWidthStyle = BoxWidthStyle.tight}) {
return _getBoxesForRange(start, end, boxHeightStyle.index, boxWidthStyle.index);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert that the arguments aren't null (and mention in the docs that they must not be null)

@Hixie
Copy link
Contributor

Hixie commented Oct 24, 2018

LGTM

@GaryQian GaryQian merged commit 9f2e2ba into flutter:master Oct 24, 2018
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 25, 2018
flutter/engine@2586e94...9f2e2ba

git log 2586e94..9f2e2ba --no-merges --oneline
9f2e2ba Add/expose API for Paragraph.getBoxesForRange BoxHeightStyle and BoxWidthStyle. (flutter/engine#6644)
9669b70 Add an iOS PlatformViewsController for creating/disposing UIViews. (flutter/engine#6569)
6c2477b fix setter for viewOpaque (flutter/engine#6653)
2bd8c94 Roll src/third_party/skia a7682c8c73e4..4f598e8c829a (8 commits) (flutter/engine#6654)
e1e6093 Realize kernel asset mappings on a worker thread if one is available. (flutter/engine#6648)
d3874a9 Roll src/third_party/skia 3b57279fd65a..a7682c8c73e4 (9 commits) (flutter/engine#6652)
3dddf4d Roll src/third_party/skia 8429c7930291..3b57279fd65a (2 commits) (flutter/engine#6650)
7b3bb62 Roll src/third_party/skia 3aca39df7b22..8429c7930291 (5 commits) (flutter/engine#6647)
26e8840 Dart SDK roll for 2018-10-23
9829c51 Roll src/third_party/skia b46c4d0925ad..3aca39df7b22 (12 commits) (flutter/engine#6643)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/&#43;/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC&#39;d on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 25, 2018
flutter/engine@2586e94...914551b

git log 2586e94..914551b --no-merges --oneline
914551b Dart SDK roll for 2018-10-24
9f2e2ba Add/expose API for Paragraph.getBoxesForRange BoxHeightStyle and BoxWidthStyle. (flutter/engine#6644)
9669b70 Add an iOS PlatformViewsController for creating/disposing UIViews. (flutter/engine#6569)
6c2477b fix setter for viewOpaque (flutter/engine#6653)
2bd8c94 Roll src/third_party/skia a7682c8c73e4..4f598e8c829a (8 commits) (flutter/engine#6654)
e1e6093 Realize kernel asset mappings on a worker thread if one is available. (flutter/engine#6648)
d3874a9 Roll src/third_party/skia 3b57279fd65a..a7682c8c73e4 (9 commits) (flutter/engine#6652)
3dddf4d Roll src/third_party/skia 8429c7930291..3b57279fd65a (2 commits) (flutter/engine#6650)
7b3bb62 Roll src/third_party/skia 3aca39df7b22..8429c7930291 (5 commits) (flutter/engine#6647)
26e8840 Dart SDK roll for 2018-10-23
9829c51 Roll src/third_party/skia b46c4d0925ad..3aca39df7b22 (12 commits) (flutter/engine#6643)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/&#43;/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC&#39;d on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 25, 2018
flutter/engine@2586e94...ff0525f

git log 2586e94..ff0525f --no-merges --oneline
ff0525f Add missing entry-points. (flutter/engine#6634)
08771fe Revert dart before engine commit 26e8840 (flutter/engine#6656)
914551b Dart SDK roll for 2018-10-24
9f2e2ba Add/expose API for Paragraph.getBoxesForRange BoxHeightStyle and BoxWidthStyle. (flutter/engine#6644)
9669b70 Add an iOS PlatformViewsController for creating/disposing UIViews. (flutter/engine#6569)
6c2477b fix setter for viewOpaque (flutter/engine#6653)
2bd8c94 Roll src/third_party/skia a7682c8c73e4..4f598e8c829a (8 commits) (flutter/engine#6654)
e1e6093 Realize kernel asset mappings on a worker thread if one is available. (flutter/engine#6648)
d3874a9 Roll src/third_party/skia 3b57279fd65a..a7682c8c73e4 (9 commits) (flutter/engine#6652)
3dddf4d Roll src/third_party/skia 8429c7930291..3b57279fd65a (2 commits) (flutter/engine#6650)
7b3bb62 Roll src/third_party/skia 3aca39df7b22..8429c7930291 (5 commits) (flutter/engine#6647)
26e8840 Dart SDK roll for 2018-10-23
9829c51 Roll src/third_party/skia b46c4d0925ad..3aca39df7b22 (12 commits) (flutter/engine#6643)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/&#43;/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC&#39;d on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 25, 2018
flutter/engine@2586e94...a6e816b

git log 2586e94..a6e816b --no-merges --oneline
a6e816b Roll src/third_party/skia 4f598e8c829a..dfca8f6adb6b (7 commits) (flutter/engine#6658)
ff0525f Add missing entry-points. (flutter/engine#6634)
08771fe Revert dart before engine commit 26e8840 (flutter/engine#6656)
914551b Dart SDK roll for 2018-10-24
9f2e2ba Add/expose API for Paragraph.getBoxesForRange BoxHeightStyle and BoxWidthStyle. (flutter/engine#6644)
9669b70 Add an iOS PlatformViewsController for creating/disposing UIViews. (flutter/engine#6569)
6c2477b fix setter for viewOpaque (flutter/engine#6653)
2bd8c94 Roll src/third_party/skia a7682c8c73e4..4f598e8c829a (8 commits) (flutter/engine#6654)
e1e6093 Realize kernel asset mappings on a worker thread if one is available. (flutter/engine#6648)
d3874a9 Roll src/third_party/skia 3b57279fd65a..a7682c8c73e4 (9 commits) (flutter/engine#6652)
3dddf4d Roll src/third_party/skia 8429c7930291..3b57279fd65a (2 commits) (flutter/engine#6650)
7b3bb62 Roll src/third_party/skia 3aca39df7b22..8429c7930291 (5 commits) (flutter/engine#6647)
26e8840 Dart SDK roll for 2018-10-23
9829c51 Roll src/third_party/skia b46c4d0925ad..3aca39df7b22 (12 commits) (flutter/engine#6643)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/&#43;/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC&#39;d on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Oct 25, 2018
flutter/engine@2586e94...a6e816b

git log 2586e94..a6e816b --no-merges --oneline
a6e816b Roll src/third_party/skia 4f598e8c829a..dfca8f6adb6b (7 commits) (flutter/engine#6658)
ff0525f Add missing entry-points. (flutter/engine#6634)
08771fe Revert dart before engine commit 26e8840 (flutter/engine#6656)
914551b Dart SDK roll for 2018-10-24
9f2e2ba Add/expose API for Paragraph.getBoxesForRange BoxHeightStyle and BoxWidthStyle. (flutter/engine#6644)
9669b70 Add an iOS PlatformViewsController for creating/disposing UIViews. (flutter/engine#6569)
6c2477b fix setter for viewOpaque (flutter/engine#6653)
2bd8c94 Roll src/third_party/skia a7682c8c73e4..4f598e8c829a (8 commits) (flutter/engine#6654)
e1e6093 Realize kernel asset mappings on a worker thread if one is available. (flutter/engine#6648)
d3874a9 Roll src/third_party/skia 3b57279fd65a..a7682c8c73e4 (9 commits) (flutter/engine#6652)
3dddf4d Roll src/third_party/skia 8429c7930291..3b57279fd65a (2 commits) (flutter/engine#6650)
7b3bb62 Roll src/third_party/skia 3aca39df7b22..8429c7930291 (5 commits) (flutter/engine#6647)
26e8840 Dart SDK roll for 2018-10-23
9829c51 Roll src/third_party/skia b46c4d0925ad..3aca39df7b22 (12 commits) (flutter/engine#6643)


The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/&#43;/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC&#39;d on the roll, and stop the roller if necessary.
Xavjer pushed a commit to Xavjer/flutter that referenced this pull request Nov 1, 2018
flutter/engine@2586e94...a6e816b

git log 2586e94..a6e816b --no-merges --oneline
a6e816b Roll src/third_party/skia 4f598e8c829a..dfca8f6adb6b (7 commits) (flutter/engine#6658)
ff0525f Add missing entry-points. (flutter/engine#6634)
08771fe Revert dart before engine commit 26e8840 (flutter/engine#6656)
914551b Dart SDK roll for 2018-10-24
9f2e2ba Add/expose API for Paragraph.getBoxesForRange BoxHeightStyle and BoxWidthStyle. (flutter/engine#6644)
9669b70 Add an iOS PlatformViewsController for creating/disposing UIViews. (flutter/engine#6569)
6c2477b fix setter for viewOpaque (flutter/engine#6653)
2bd8c94 Roll src/third_party/skia a7682c8c73e4..4f598e8c829a (8 commits) (flutter/engine#6654)
e1e6093 Realize kernel asset mappings on a worker thread if one is available. (flutter/engine#6648)
d3874a9 Roll src/third_party/skia 3b57279fd65a..a7682c8c73e4 (9 commits) (flutter/engine#6652)
3dddf4d Roll src/third_party/skia 8429c7930291..3b57279fd65a (2 commits) (flutter/engine#6650)
7b3bb62 Roll src/third_party/skia 3aca39df7b22..8429c7930291 (5 commits) (flutter/engine#6647)
26e8840 Dart SDK roll for 2018-10-23
9829c51 Roll src/third_party/skia b46c4d0925ad..3aca39df7b22 (12 commits) (flutter/engine#6643)


The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/&#43;/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC&flutter#39;d on the roll, and stop the roller if necessary.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants