Skip to content

Conversation

manc88
Copy link

@manc88 manc88 commented Nov 2, 2018

Hi, add StatefulWidget for details tag.

@Sub6Resources
Copy link
Owner

Thank you for the pull request. I will review this soon!

@Sub6Resources Sub6Resources added the enhancement New feature or request label Nov 3, 2018
@Sub6Resources Sub6Resources added this to the 0.9.0 milestone Nov 3, 2018
@Sub6Resources Sub6Resources self-requested a review November 3, 2018 01:07
@Sub6Resources Sub6Resources self-assigned this Nov 3, 2018
Copy link
Owner

@Sub6Resources Sub6Resources left a comment

Choose a reason for hiding this comment

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

These are some good changes. Thanks for the pull request! Just a couple additional things to make this a tad more maintainable.


typedef CustomRender = Widget Function(dom.Node node, List<Widget> children);
typedef OnLinkTap = void Function(String url);
typedef ParseNodeFunction = List<Widget> Function(List<dom.Node> nodeList);
Copy link
Owner

Choose a reason for hiding this comment

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

Rather than creating a custom callback to parse widgets, just pass in the list of children Widgets to the Widget.

Copy link
Author

Choose a reason for hiding this comment

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

I left it after refactoring because it is also needed to build

node content, but we cannot know in advance if there is a "summary" tag, or have to determine the content of the tag outside the class, which seems to me not the best way to structure the code.

@Sub6Resources
Copy link
Owner

This package has changed a great deal and will continue to change a great deal since you first opened this pull request. I'm going to close this pull request for now. If you are willing to make the changes to make this compatible with the most recent version, including the RichText parser, feel free to push your changes and I'll gladly open this again.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants