Skip to content

[BUG] Top margin gets set to '[text]' margin #1155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
arjanmels opened this issue Oct 2, 2022 · 6 comments · Fixed by #1237
Closed

[BUG] Top margin gets set to '[text]' margin #1155

arjanmels opened this issue Oct 2, 2022 · 6 comments · Fixed by #1237
Labels
bug Something isn't working whitespace issues

Comments

@arjanmels
Copy link
Contributor

arjanmels commented Oct 2, 2022 •
edited by Sub6Resources
Loading

Describe the bug:
The '[text]' elements get a bottom of margin of 32 in my case.
In the _collapseMargins routine this gets collapsed into the next elements margin.

My solution/workaround add a Style to the style map with key '[text]' setting sthe margins to 0.

HTML to reproduce the issue:

<p>test1</p>
<p>test2</p>

Html widget configuration:

Html.fromDom(
            document: _document,
            style: buildContext.styles.htmlStyle,
            customRenders: {
              tableMatcher(): tableRender(),
              _summaryMatcher(): CustomRender.inlineSpan(inlineSpan: (context, buildChildren) {
                var style = context.tree.children.first.style;
                style.color = null;
                var textStyle = style.generateTextStyle();
                return WidgetSpan(child: Text(context.tree.element?.text ?? '', style: textStyle));
              }),
            },
          ),

Expected behavior:
No margin on [text] element by default

Screenshots:
N/A

Device details and Flutter/Dart/flutter_html versions:
Version: 3.0.0-alpha6

Stacktrace/Logcat
N/A
Additional info:
N/A
@arjanmels arjanmels added the bug Something isn't working label Oct 2, 2022
@Sub6Resources
Copy link
Owner

This is certainly odd. Can you give us an example of HTML that can reproduce this issue?

@Sub6Resources Sub6Resources added the more-info-needed More information is needed to resolve the issue. Will be closed if no response is received. label Oct 3, 2022
@arjanmels
Copy link
Contributor Author

Structure of the html is:

<html lang="en">
<body>
<details>
    <summary><h1>Header 1</h1></summary>
    <details>
        <summary><h2>Hoe slapen werkt</h2></summary>
        <h3>Header 3</h3>
        <p>Paragraph 1</p>
        <h4>Header 4</h4>
        <p>Paragraph 2</p>
        <p>Paragraph 3</p>
    </details>
</details>
</body>
</html>

I can image that the style setup is also important. Unfortunately not entirely straightforward how I create it:

In this setup I already included my workaround with the '[text]' style

Style _createHTMLStyle(final TextStyle? textStyle,
      {FontWeight? fontWeight, double top = 0.5, double bottom = 0.25, double horizontal = 17}) {
    if (textStyle == null) {
      return Style(
          margin: Margins(
              top: Margin(top, Unit.rem),
              bottom: Margin(bottom, Unit.rem),
              left: Margin(horizontal, Unit.px),
              right: Margin(horizontal, Unit.px)));
    } else {
      return Style.fromTextStyle(textStyle).copyWith(fontWeight: fontWeight).copyWith(
          margin: Margins(
              top: Margin(top, Unit.rem),
              bottom: Margin(bottom, Unit.rem),
              left: Margin(horizontal, Unit.px),
              right: Margin(horizontal, Unit.px)));
    }
  }

  Map<String, Style> get htmlStyle {
    var textTheme = _context.theme.textTheme;
    return {
      'body': Style(margin: Margins.zero),
      'h1': _createHTMLStyle(textTheme.headlineMedium),
      'h2': _createHTMLStyle(textTheme.headlineSmall),
      'h3': _createHTMLStyle(textTheme.titleSmall, fontWeight: FontWeight.bold),
      'h4': _createHTMLStyle(textTheme.bodyMedium, fontWeight: FontWeight.bold),
      'h5': _createHTMLStyle(textTheme.bodyMedium),
      'h6': _createHTMLStyle(textTheme.bodyMedium),
      'p': _createHTMLStyle(textTheme.bodyMedium, top: 0, bottom: 1),
      'ul, ol': _createHTMLStyle(null, top: 0, bottom: 1),
      'table': _createHTMLStyle(null, top: 0, bottom: 1, horizontal: 17 / 2)
          .copyWith(padding: EdgeInsets.only(bottom: _scale * textTheme.bodyMedium!.fontSize! / 4)),
      'tr': Style(margin: Margins.only(right: -6)),
      'th': Style(fontWeight: FontWeight.bold),
      'td': Style(padding: const EdgeInsets.only(right: 3, bottom: 1.5), alignment: Alignment.topLeft),
      'details': Style(),
      'summary': Style(),
      '[text]': Style(margin: Margins.zero)
    };
  }

@Sub6Resources
Copy link
Owner

Thanks, This will take me a bit to work through but I'll take a look.

@Sub6Resources Sub6Resources added in-triage Issue's that I've seen but haven't had a chance to thoroughly review and/or categorize and removed more-info-needed More information is needed to resolve the issue. Will be closed if no response is received. labels Oct 3, 2022
@arjanmels
Copy link
Contributor Author

My workaround fails when there is no empty text element between tags.
E.g. This html

<!DOCTYPE html>
<html>
<body>
    <div>
        <p>a</p><p>b</p>
        <p>d</p>
        <p>a</p><p>b</p>
    </div>
</body>
</html>

leads to this output:

a

b
d
a

b

@arjanmels arjanmels mentioned this issue Mar 4, 2023
Merged
@arjanmels
Copy link
Contributor Author

I think I have found the problem. Margin was added both to bottom of previous block and top of next block.
Pull Request #1237 should address it.

@Sub6Resources
Copy link
Owner

@arjanmels Thanks for catching that!

@Sub6Resources Sub6Resources added whitespace issues and removed in-triage Issue's that I've seen but haven't had a chance to thoroughly review and/or categorize labels May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working whitespace issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Correct margin collapse noviocare/flutter_html
2 participants
@Sub6Resources @arjanmels