Skip to content
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.

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

Added support for rendering as RichText Widgets #37

Merged
merged 6 commits into from Feb 1, 2019

Conversation

@jeffmikels
Copy link
Contributor

jeffmikels commented Nov 6, 2018

The original HTML Parser rendered widgets awkwardly. I have implemented a parser that converts the HTML into a list of Rich Text Widgets with appropriate TextSpans.

@Sub6Resources

This comment has been minimized.

Copy link
Owner

Sub6Resources commented Nov 6, 2018

Thank you for the extensive pull request! I'll review this as soon as I am have some time (because this is somewhat of a major change it may take some time).

@Sub6Resources Sub6Resources self-requested a review Nov 6, 2018
@Sub6Resources Sub6Resources self-assigned this Nov 6, 2018
@Kapranov98 Kapranov98 mentioned this pull request Nov 7, 2018
@jeffmikels

This comment has been minimized.

Copy link
Contributor Author

jeffmikels commented Nov 9, 2018

Thank you for the extensive pull request! I'll review this as soon as I am have some time (because this is somewhat of a major change it may take some time).

To be clear on the changes:

  • I have renamed the HtmlParser to HtmlOldParser, but it remains the default parser and currently supports more tags
  • The new parser is called HtmlRichTextParser, and can be selected by setting the useRichText flag when creating the Html widget.
  • The new parser doesn't support as many tags as the old one, but it renders them more predictably.
  • The new parser does NOT support the specification of a customRender function.
  • The supported HTML tags are easily seen in the code.
@wwwdata

This comment has been minimized.

Copy link
Contributor

wwwdata commented Jan 9, 2019

I now also noticed the problem when having links inside the HTML #40 , that they are getting rendered on a new line with the old parser. However I would need to still have the ability to use custom renderings. I would be willing to look into how to implement this feature.

@Sub6Resources can you review this and merge?

I will start using this as a base now and see how far I can get implementing the ability to use the custom renderer again. Thank you so much for solving this issue @jeffmikels

@hui00

This comment has been minimized.

Copy link

hui00 commented Jan 18, 2019

with some data and useRichText: true

NoSuchMethodError: The getter 'isEmpty' was called on null. Reveiver: null Tried calling: isEmpty

@jeffmikels

This comment has been minimized.

Copy link
Contributor Author

jeffmikels commented Jan 19, 2019

with some data and useRichText: true

NoSuchMethodError: The getter 'isEmpty' was called on null. Reveiver: null Tried calling: isEmpty

My latest commit fixes this.

@skipness

This comment has been minimized.

Copy link

skipness commented Jan 29, 2019

I am trying to make nested blockquote like this:
<blockquote><blockquote>2</blockquote>1</blockquote><br /><h1>heading</h1>
works. I get the following result:
image
Seems the new parser doesn't support nested blockquote tag with non block tag inside?

@jeffmikels

This comment has been minimized.

Copy link
Contributor Author

jeffmikels commented Jan 29, 2019

I am trying to make nested blockquote like this:
<blockquote><blockquote>2</blockquote>1</blockquote><br /><h1>heading</h1>
works. I get the following result:
image
Seems the new parser doesn't support nested blockquote tag with non block tag inside?

What behavior are you expecting?

@skipness

This comment has been minimized.

Copy link

skipness commented Jan 30, 2019

I am trying to make nested blockquote like this:
<blockquote><blockquote>2</blockquote>1</blockquote><br /><h1>heading</h1>
works. I get the following result:
image
Seems the new parser doesn't support nested blockquote tag with non block tag inside?

What behavior are you expecting?

image
like this

@jeffmikels

This comment has been minimized.

Copy link
Contributor Author

jeffmikels commented Jan 31, 2019

@skipness

I have tweaked the code to treat plain text nodes inside blockquote, ol, and ul elements as if they had been block elements. This will properly indent them. However, there is a limitation to how this rendering is done, and you might not be pleased with the results as they are different from what you expect.

My renderer creates a series of "BlockText" widgets in a column. BlockText Widgets cannot contain other BlockText widgets, so the way the renderer works is that when blockquote, ol, or ul elements are encountered, a flag is set telling the renderer that subsequent block widgets should be indented and might need a left side border.

As a result, this renderer is only capable of handling a small subset of html correctly. For example, block elements inside other block elements will not behave as expected.

To more effectively handle nested block elements, I will need to modify my BlockText widgets to handle multiple children. That has not been done yet.

Copy link
Owner

Sub6Resources left a comment

Thank you for your patience and for your work on this pull request. I feel good about merging this. Once this is merged and I've updated the CHANGELOG and README, I'll release version 0.9.0.

@Sub6Resources Sub6Resources merged commit c4220df into Sub6Resources:master Feb 1, 2019
@Sub6Resources

This comment has been minimized.

Copy link
Owner

Sub6Resources commented Feb 1, 2019

These changes have been published as version 0.9.0. Thank you @jeffmikels for your work on this.

@skipness

This comment has been minimized.

Copy link

skipness commented Feb 1, 2019

@Sub6Resources @jeffmikels thank you very much!

@mesuutt

This comment has been minimized.

Copy link

mesuutt commented Feb 7, 2019

@jeffmikels how can we use customRender with useRichText? Why new parser not support customRender?

@jeffmikels

This comment has been minimized.

Copy link
Contributor Author

jeffmikels commented Feb 7, 2019

I didn't understand the implementation of customRender, so I didn't implement it. The new parser has a RichText customRenderer built in.

@mesuutt

This comment has been minimized.

Copy link

mesuutt commented Feb 8, 2019

I want to add different style to some of html elements, I achieved it with customRender but old parser not generate correct widget tree and has some issues. How am I achieve what described in #53?

@wwwdata

This comment has been minimized.

Copy link
Contributor

wwwdata commented Feb 8, 2019

@mesuutt it's not implemented yet. We need some custom rendering as well so I already played around but what I currently have is not really a good solution. I can look into it in the next days and open a PR when I have something.

If you need it right now you could also just fork it and add your rendering in the new code for now until there is an extension mechanism

@Sub6Resources

This comment has been minimized.

Copy link
Owner

Sub6Resources commented Feb 8, 2019

@mesuutt I've opened a new issue so we can track the progress of this feature here: #64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.