-
-
Notifications
You must be signed in to change notification settings - Fork 932
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Fix cascading and fallback propagation of text styles (#3129)
Fix cascading and fallback propagation of text styles. While using text nodes and flame_markdown, I noticed that basic propagating of styles was not working. For example, if you create a standard document, it will have some margins by default. Even when setting it to zero, it was still overwritten by the default style. I noticed that there was no consistency in the methods `copyWith` and `merge` of which parameter/receiver should be the master and which should be the fallback. Both the implementations and the callers would use it either way indiscriminately. This consolidates the definition and adds comments to the methods enforcing the priority order. Also fixes the wrong precedence on `copyWith` methods. In order to fully test this, I had to expose a few internal details of text elements; I think this is fine since Dart supports the `@visibleForTesting` annotation, and it makes the tests cleaner.
- Loading branch information
1 parent
6b63a57
commit 7b706d5
Showing
6 changed files
with
189 additions
and
25 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| import 'package:flame/src/text/elements/group_element.dart'; | ||
| import 'package:flame/src/text/elements/group_text_element.dart'; | ||
| import 'package:flame/text.dart'; | ||
| import 'package:flutter/rendering.dart'; | ||
| import 'package:test/test.dart'; | ||
|
|
||
| void main() { | ||
| group('text styles', () { | ||
| test('document style defaults are applied', () { | ||
| final style = DocumentStyle(); | ||
| expect(style.text.fontSize, 16); | ||
| expect(style.boldText.fontWeight, FontWeight.bold); | ||
| expect(style.italicText.fontStyle, FontStyle.italic); | ||
| expect(style.paragraph.margin, const EdgeInsets.all(6)); | ||
| expect(style.paragraph.padding, EdgeInsets.zero); | ||
| }); | ||
|
|
||
| test('document style parameters are respected', () { | ||
| final style = DocumentStyle( | ||
| text: InlineTextStyle( | ||
| fontSize: 8, | ||
| ), | ||
| boldText: InlineTextStyle( | ||
| fontWeight: FontWeight.w900, | ||
| ), | ||
| italicText: InlineTextStyle( | ||
| fontStyle: FontStyle.normal, | ||
| ), | ||
| paragraph: const BlockStyle( | ||
| margin: EdgeInsets.zero, | ||
| padding: EdgeInsets.zero, | ||
| ), | ||
| ); | ||
| expect(style.text.fontSize, 8); | ||
| expect(style.boldText.fontWeight, FontWeight.w900); | ||
| expect(style.italicText.fontStyle, FontStyle.normal); | ||
| expect(style.paragraph.margin, EdgeInsets.zero); | ||
| expect(style.paragraph.padding, EdgeInsets.zero); | ||
| }); | ||
|
|
||
| test('document style can be copied', () { | ||
| final style = DocumentStyle( | ||
| text: InlineTextStyle( | ||
| fontSize: 8, | ||
| ), | ||
| boldText: InlineTextStyle( | ||
| fontWeight: FontWeight.w900, | ||
| ), | ||
| paragraph: const BlockStyle( | ||
| margin: EdgeInsets.zero, | ||
| padding: EdgeInsets.zero, | ||
| ), | ||
| background: BackgroundStyle( | ||
| color: const Color(0xFFFF00FF), | ||
| ), | ||
| ); | ||
| final newStyle = style.copyWith( | ||
| DocumentStyle( | ||
| text: InlineTextStyle( | ||
| fontSize: 10, | ||
| ), | ||
| boldText: InlineTextStyle( | ||
| fontWeight: FontWeight.w900, | ||
| ), | ||
| italicText: InlineTextStyle( | ||
| fontStyle: FontStyle.italic, | ||
| ), | ||
| paragraph: const BlockStyle( | ||
| margin: EdgeInsets.all(10), | ||
| padding: EdgeInsets.zero, | ||
| ), | ||
| ), | ||
| ); | ||
| expect(newStyle.text.fontSize, 10); | ||
| expect(newStyle.boldText.fontWeight, FontWeight.w900); | ||
| expect(newStyle.italicText.fontStyle, FontStyle.italic); | ||
| expect(newStyle.paragraph.margin, const EdgeInsets.all(10)); | ||
| expect(newStyle.paragraph.padding, EdgeInsets.zero); | ||
| expect( | ||
| newStyle.background!.backgroundPaint!.color, | ||
| const Color(0xFFFF00FF), | ||
| ); | ||
| }); | ||
|
|
||
| test('styles are cascading', () { | ||
| final style = DocumentStyle( | ||
| width: 600.0, | ||
| height: 400.0, | ||
| text: InlineTextStyle( | ||
| fontFamily: 'Helvetica', | ||
| fontSize: 8, | ||
| ), | ||
| boldText: InlineTextStyle( | ||
| fontFamily: 'Arial', | ||
| fontSize: 10, | ||
| fontWeight: FontWeight.w900, | ||
| ), | ||
| italicText: InlineTextStyle( | ||
| fontFamily: 'Arial', | ||
| fontSize: 6, | ||
| fontStyle: FontStyle.italic, | ||
| ), | ||
| paragraph: BlockStyle( | ||
| text: InlineTextStyle( | ||
| fontSize: 12, | ||
| ), | ||
| ), | ||
| ); | ||
| final document = DocumentRoot([ | ||
| ParagraphNode.group([ | ||
| PlainTextNode('This is '), | ||
| BoldTextNode.simple('my'), | ||
| PlainTextNode(' town - '), | ||
| ItalicTextNode.simple('The Sheriff'), | ||
| ]), | ||
| ]); | ||
|
|
||
| final element = document.format(style); | ||
| final groupElement = element.children.first as GroupElement; | ||
| final groupTextElement = groupElement.children.first as GroupTextElement; | ||
| final styles = groupTextElement.children.map((e) { | ||
| return (e as TextPainterTextElement).textPainter.text!.style!; | ||
| }).toList(); | ||
|
|
||
| expect(styles[0].fontSize, 12); | ||
| expect(styles[0].fontWeight, isNull); | ||
| expect(styles[0].fontStyle, isNull); | ||
| expect(styles[0].fontFamily, 'Helvetica'); | ||
|
|
||
| expect(styles[1].fontSize, 10); | ||
| expect(styles[1].fontWeight, FontWeight.w900); | ||
| expect(styles[1].fontStyle, isNull); | ||
| expect(styles[1].fontFamily, 'Arial'); | ||
|
|
||
| expect(styles[2].fontSize, 12); | ||
| expect(styles[2].fontWeight, isNull); | ||
| expect(styles[2].fontStyle, isNull); | ||
| expect(styles[2].fontFamily, 'Helvetica'); | ||
|
|
||
| expect(styles[3].fontSize, 6); | ||
| expect(styles[3].fontWeight, isNull); | ||
| expect(styles[3].fontStyle, FontStyle.italic); | ||
| expect(styles[3].fontFamily, 'Arial'); | ||
| }); | ||
| }); | ||
| } |