Skip to content

Commit

Permalink
Fix infinite TextStyle.height (#1147)
Browse files Browse the repository at this point in the history
Also:

* Build text style with line height in `InheritedProperties`
* Deprecate `InheritedProperties.style` getter
  • Loading branch information
daohoangson authored Jan 6, 2024
1 parent eafd747 commit a4f734d
Show file tree
Hide file tree
Showing 26 changed files with 346 additions and 204 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
This repo contains the source code for everything `HtmlWidget`-related.

| Name | Link |
|--------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------|
| ------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------- |
| [flutter_widget_from_html_core](./packages/core/) | [![pub package](https://img.shields.io/pub/v/flutter_widget_from_html_core.svg)](https://pub.dev/packages/flutter_widget_from_html_core) |
| [flutter_widget_from_html](./packages/enhanced/) | [![pub package](https://img.shields.io/pub/v/flutter_widget_from_html.svg)](https://pub.dev/packages/flutter_widget_from_html) |
| [fwfh_cached_network_image](./packages/fwfh_cached_network_image/) | [![pub package](https://img.shields.io/pub/v/fwfh_cached_network_image.svg)](https://pub.dev/packages/fwfh_cached_network_image) |
Expand Down
Binary file modified demo_app/test/sizing/_guessChildSize/sized_inline_block.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 13 additions & 11 deletions docs/extensibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ HtmlWidget(
},
),
```

[Try with fwfh.dev](https://try.fwfh.dev/?id=08173c5e5d837293837c383d00f9f792)

</td>
Expand All @@ -51,8 +51,9 @@ This example renders a carousel ([live demo](https://demo.fwfh.dev/#/customwidge
<img src="https://raw.githubusercontent.com/daohoangson/flutter_widget_from_html/bd80e2fef38f8d7ed69c388e2b325ea09aa7b817/demo_app/screenshots/CustomWidgetBuilderScreen.gif" width="300" />

Notes:
- By default, the custom widget will take the full width
- Wrap it in `InlineCustomWidget` to inline with surrounding text

- By default, the custom widget will take the full width
- Wrap it in `InlineCustomWidget` to inline with surrounding text

# Custom factory

Expand Down Expand Up @@ -87,7 +88,7 @@ flowchart TD
-->|yes\n\ncustomWidgetBuilder| ifIsInlineCustomWidget{inline\nwidget?}
--->|no| appendWidgetBitBlock[/render\nblock widget/]
---> bitOK
ifIsInlineCustomWidget
-->|yes\n\nInlineCustomWidget| flattener
-->|onRenderInline| RichText[/render\nRichText/]
Expand All @@ -113,15 +114,15 @@ flowchart TD

You can modify inherited properties, including text styles, using the `inherit` function and registering your resolver callback to be invoked once the `BuildContext` is prepared. Here's how it works:

- Your callback will be supplied with two parameters.
- The first parameter is an immutable `InheritedProperties` object, calculated from the root to each element. To make changes, your callback must return a new `InheritedProperties` by calling `copyWith`. If no changes are required, it's recommended to return the same object.
- Additionally, you can pass a dynamic value when you call `inherit`, and your callback will receive it as the second parameter during execution.
- Your callback will be supplied with two parameters.
- The first parameter is an immutable `InheritedProperties` object, calculated from the root to each element. To make changes, your callback must return a new `InheritedProperties` by calling `copyWith`. If no changes are required, it's recommended to return the same object.
- Additionally, you can pass a dynamic value when you call `inherit`, and your callback will receive it as the second parameter during execution.

```dart
// simple resolver setting text color
tree.inherit(
(resolving, _) => resolving.copyWith(
style: resolving.style.copyWith(
style: TextStyle(
color: Colors.red,
),
),
Expand All @@ -130,7 +131,7 @@ tree.inherit(
// resolver uses the second param to set height
tree.inherit(
(resolving, height) => resolving.copyWith(
style: resolving.style.copyWith(
style: TextStyle(
height: height,
),
),
Expand All @@ -139,11 +140,12 @@ tree.inherit(
```

Notes:
- Use the `resolving.copyWith<Foo>(value: foo)` method to store various data types within the tree. Children elements can access this value through `resolved.get<Foo>()`.

- Use the `resolving.copyWith<Foo>(value: foo)` method to store various data types within the tree. Children elements can access this value through `resolved.get<Foo>()`.

## Build ops

Complex functionalities can be implemented using `BuildOp` or `BuildOp.inline`. Internally, all of the HTML tags are supported with the same architecture. This means that by adopting this method, your potential for customization is virtually limitless. If you encounter any obstacles while trying to create custom rendering, please don't hesitate to open a GitHub enhancement issue.
Complex functionalities can be implemented using `BuildOp` or `BuildOp.inline`. Internally, all of the HTML tags are supported with the same architecture. This means that by adopting this method, your potential for customization is virtually limitless. If you encounter any obstacles while trying to create custom rendering, please don't hesitate to open a GitHub enhancement issue.

```dart
tree.register(BuildOp(
Expand Down
32 changes: 32 additions & 0 deletions docs/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,38 @@ In this version, we've introduce a new factory to simplify the rendering of cust

The entire `TextStyleHtml` class has been marked as deprecated, its successor is the `InheritedProperties` class. However, if you have legacy code using `TextStyleHtml`, it will continue to work.

### style

Use `prepareTextStyle` to obtain a `TextStyle` object for custom spans or widgets. This update is to improve efficiency, as the `style` getter method had to account for line height, making repeated calls inefficient.

In the past, this getter was essential for modifying certain text style attributes. However, this is no longer necessary. The updated `InheritedProperties.copyWith` method merges changes rather than directly replacing the current style.

<table><tr><th>Before</th><th>After</th></tr><tr><td>

```dart
tree.inherit(
(resolving, _) => resolving.copyWith(
style: resolving.style.copyWith(
color: Colors.red,
),
),
);
```

</td><td>

```dart
tree.inherit(
(resolving, _) => resolving.copyWith(
style: TextStyle(
color: Colors.red,
),
),
);
```

</td></tr></table>

### getDependency

Similarly, the `getDependency` method has been deprecated, and you should use `get` to retrieve values by their type. It's worth noting that, for performance reasons, `v0.14` no longer depends on `MediaQueryData` to avoid excessive rebuild. Therefore, attempting to use either `getDependency<MediaQueryData>` or `get<MediaQueryData>` will not work.
3 changes: 2 additions & 1 deletion packages/core/lib/src/core_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ part 'data/build_op.dart';
part 'data/css.dart';
part 'data/image.dart';
part 'data/inherited_properties.dart';
part 'data/line_height.dart';
part 'data/lockable_list.dart';
part 'data/non_inherited_properties.dart';
part 'data/normal_line_height.dart';
part 'data/text_scale_factor.dart';
1 change: 0 additions & 1 deletion packages/core/lib/src/core_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import 'core_widget_factory.dart';
import 'widgets/inline_custom_widget.dart';

export 'external/csslib.dart';
export 'external/text_scale_factor.dart';
export 'modes/render_mode.dart';
export 'widgets/css_sizing.dart';
export 'widgets/horizontal_margin.dart';
Expand Down
5 changes: 5 additions & 0 deletions packages/core/lib/src/core_legacy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ extension LegacyWidgetFactory on WidgetFactory {
typedef TextStyleHtml = InheritedProperties;

extension LegacyTextStyleHtml on TextStyleHtml {
/// The [TextStyle].
@Deprecated('Use `prepareTextStyle` to build one. '
'For usage in resolving.copyWith, check the migration guide.')
TextStyle get style => prepareTextStyle();

/// Gets dependency by type [T].
@Deprecated('Use .get instead.')
T getDependency<T>() {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/lib/src/core_widget_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ class WidgetFactory extends WidgetFactoryResetter with AnchorWidgetFactory {
int index,
) {
final text = getListMarkerText(listStyleType, index);
final textStyle = resolved.style;
final textStyle = resolved.prepareTextStyle();
if (text.isNotEmpty) {
return buildText(tree, resolved, TextSpan(style: textStyle, text: text));
}
Expand Down
4 changes: 2 additions & 2 deletions packages/core/lib/src/data/css.dart
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class CssBorderSide {
return null;
}

final scopedColor = color ?? resolved.style.color;
final scopedColor = color ?? resolved.get<TextStyle>()?.color;
if (scopedColor == null) {
return null;
}
Expand Down Expand Up @@ -266,7 +266,7 @@ class CssLength {
case CssLengthUnit.auto:
return null;
case CssLengthUnit.em:
baseValue ??= resolved.style.fontSize;
baseValue ??= resolved.get<TextStyle>()?.fontSize;
if (baseValue == null) {
return null;
}
Expand Down
88 changes: 71 additions & 17 deletions packages/core/lib/src/data/inherited_properties.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,11 @@ class InheritedProperties {
/// The parent set.
final InheritedProperties? parent;

/// The [TextStyle].
final TextStyle style;

final Iterable<dynamic> values;

const InheritedProperties(
this.values, {
this.parent,
this.style = const TextStyle(),
});
final TextStyle _style;

const InheritedProperties._(this.parent, this.values, this._style);

/// Creates the root properties set.
factory InheritedProperties.root([
Expand All @@ -36,12 +31,13 @@ class InheritedProperties {
);
}

return InheritedProperties(
return InheritedProperties._(
null,
[
...deps,
NormalLineHeight(style.height),
if (style.height != null) NormalLineHeight(style.height),
],
style: style,
style,
);
}

Expand All @@ -51,18 +47,76 @@ class InheritedProperties {
TextStyle? style,
T? value,
}) {
return InheritedProperties(
var newStyle = _style;
if (style != null) {
if (style.inherit) {
newStyle = newStyle.merge(style);
} else {
newStyle = style;
}
}

return InheritedProperties._(
parent ?? this.parent,
value != null ? [...values.where((e) => e is! T), value] : values,
parent: parent ?? this.parent,
style: style ?? this.style,
newStyle,
);
}

/// Gets inherited property of type [T].
///
/// The initial set of values are populated by [WidgetFactory.getDependencies].
/// Parser and builder may use [BuildTree.inherit] to enqueue more.
T? get<T>() => _get(values);
T? get<T>() {
if (T == TextStyle) {
// by default, `get` will return the TextStyle from the original deps list
// let's intercept here and return the active object instead
// this also acts as an escape hatch to get the TextStyle directly without preparing
return _style as T;
}

return _get(values);
}

/// Prepares [TextStyle] with correct line-height.
TextStyle prepareTextStyle() {
final height = get<CssLineHeight>();
if (height == null) {
return _style;
}

final length = height.value;
if (length == null) {
final normalValue = get<NormalLineHeight>()?.value;
if (normalValue == null) {
return _style;
} else {
return _style.copyWith(
debugLabel: 'fwfh: line-height normal',
height: normalValue,
);
}
}

final fontSize = _style.fontSize;
if (fontSize == null || fontSize == .0) {
return _style;
}

final lengthValue = length.getValue(
this,
baseValue: fontSize,
scaleFactor: get<TextScaleFactor>()?.value,
);
if (lengthValue == null) {
return _style;
}

return _style.copyWith(
debugLabel: 'fwfh: line-height',
height: lengthValue / fontSize,
);
}

static T? _get<T>(Iterable<dynamic> values) {
for (final value in values.whereType<T>()) {
Expand Down Expand Up @@ -139,8 +193,8 @@ class InheritanceResolvers {
///
/// If the parent's values are unchanged, the cached resolved set will be used.
InheritedProperties resolve(BuildContext context) {
final parentResolved =
parent?.resolve(context) ?? const InheritedProperties([]);
final parentResolved = parent?.resolve(context) ??
const InheritedProperties._(null, [], TextStyle());
final scopedCallbacks = _callbacks;
if (scopedCallbacks == null) {
return parentResolved;
Expand Down
21 changes: 21 additions & 0 deletions packages/core/lib/src/data/line_height.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
part of '../core_data.dart';

/// The height of text.
///
/// See [TextStyle.height].
@immutable
class CssLineHeight {
final CssLength? value;

const CssLineHeight([this.value]);
}

/// The default height of text.
///
/// See [TextStyle.height].
@immutable
class NormalLineHeight {
final double? value;

const NormalLineHeight(this.value);
}
7 changes: 0 additions & 7 deletions packages/core/lib/src/data/normal_line_height.dart

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import 'package:flutter/material.dart';
part of '../core_data.dart';

/// The number of font pixels for each logical pixel.
///
Expand Down
4 changes: 2 additions & 2 deletions packages/core/lib/src/internal/flattener.dart
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class Flattener implements Flattened {

return wf.buildTextSpan(
recognizer: _getInlineRecognizer(context, resolved),
style: resolved.style,
style: resolved.prepareTextStyle(),
text: text,
);
},
Expand Down Expand Up @@ -233,7 +233,7 @@ class Flattener implements Flattened {
span = wf.buildTextSpan(
children: children,
recognizer: _getInlineRecognizer(context, resolved),
style: resolved.style,
style: resolved.prepareTextStyle(),
text: text,
);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/core/lib/src/internal/ops/anchor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ class Anchor {
onRenderInline: (tree) {
final widget = WidgetPlaceholder(
builder: (context, _) => SizedBox(
height: tree.inheritanceResolvers.resolve(context).style.fontSize,
height: tree.inheritanceResolvers
.resolve(context)
.get<TextStyle>()
?.fontSize,
key: anchor,
),
debugLabel: '${tree.element.localName}--anchor#$id',
Expand Down
2 changes: 1 addition & 1 deletion packages/core/lib/src/internal/ops/style_background.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class StyleBackground {
Color color,
) =>
resolving.copyWith(
style: resolving.style.copyWith(
style: TextStyle(
background: Paint()..color = color,
debugLabel: 'fwfh: $kCssBackgroundColor',
),
Expand Down
Loading

1 comment on commit a4f734d

@github-actions
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.