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

Conversation

@jason-simmons
Copy link
Member

@GaryQian
Copy link
Contributor

GaryQian commented May 9, 2019

Can you add a test in paragraph_unittests that verifies this behavior alongside the tests for each other mode?

/// {@macro flutter.dart:ui.boxHeightStyle.includeLineSpacing}
includeLineSpacingBottom,

/// Calculate box heights based on the metrics of this paragraph's [StrutStyle].
Copy link
Contributor

Choose a reason for hiding this comment

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

This could use a bit more detail about the properties of this, such as each box will have the same height, and the top/bottoms of the boxes will line up with each other.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

// The line spacing will be added to the bottom of the rect.
kIncludeLineSpacingBottom
kIncludeLineSpacingBottom,

Copy link
Contributor

Choose a reason for hiding this comment

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

extra space? not consistent with rest of the code.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is intended to separate kStrut from the group of kIncludeLine values

@jason-simmons
Copy link
Member Author

Added a test - PTAL

Copy link
Contributor

@GaryQian GaryQian left a comment

Choose a reason for hiding this comment

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

LGTM!

@jason-simmons jason-simmons merged commit aa63f09 into flutter:master May 16, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 16, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request May 16, 2019
flutter/engine@288a855...9d7cd70

git log 288a855..9d7cd70 --no-merges --oneline
9d7cd70 add observatoryUrl property to FlutterEngine (flutter/engine#8987)
34a5248 Add matrix4 param to Linear gradients (flutter/engine#8985)
aa63f09 libtxt: add a BoxHeightStyle option based on the height of the strut (flutter/engine#8927)

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/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff (jsimmons@google.com), 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.

3 participants