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

Feature request: support images inside tables #68

Closed
IlyaGulya opened this issue Sep 26, 2018 · 6 comments
Closed

Feature request: support images inside tables #68

IlyaGulya opened this issue Sep 26, 2018 · 6 comments
Milestone

Comments

@IlyaGulya
Copy link

Hello!

According to readme, library supports inline markdown: https://github.com/noties/Markwon/blame/master/README.md#L230

But it seems that it does not support images which located inside the tables in Markdown.

Here is example of such case (got it from here https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21888):

Before After
Screen_Shot_2018-09-24_at_18.08.23 Screen_Shot_2018-09-24_at_18.54.10

Using Markwon leads to such behavior:

@noties
Copy link
Owner

noties commented Sep 27, 2018

Hello @IlyaGulya !

Currently tables do not support images. This comes from the fact that TextView does not support columns. That's why table draws its content manually (you won't find table content in a TextView if you inspect getText for example). This is decision that I made long time ago. Now I think it should be addressed because (currently) I do not see why not

* FYI one cannot control the width of a table column is another limitation of tables implementation. All columns will be equal width

@noties noties added this to the 2.1.0 milestone Sep 27, 2018
@brucetoo
Copy link

brucetoo commented Jan 5, 2019

How about change the way to just render table by webview, the other tags render by textview ? Recently i finish a demo to overcome this. So we first need separate <table> with others supported tags, which will like change "<p>**<p> blabala <table>balabla </table> blabla <p>**<p> " to three separated strings

  • <p>**<p> blabala
  • <table>balabla </table>
  • blabla <p>**<p>

Then only <table> included tags render by webview, the others by textview
And the result in android will be like:

<ScrollView> 
    <LinearLayout>
        <TextView> 
        <WebView>  --- let the webview ATMOST measured
        <TextView> 

So every thing goes fine as i think. check this commit for detail

@noties
Copy link
Owner

noties commented Jan 5, 2019

Hello @brucetoo !

First of all, I want to address parent comment. I think I won't be adding images to tables in current version. I mean, it's limited, but works for simple cases. Next major release though will address that.

Now, adding a WebView (possibly multiple) inside a ScrollView will decrease performance drastically. And maybe rendering the whole thing in a WebView will be more performance wise. Also, currently we support a single TextView. Well, out-of-box. Adding management of multiple views (not only TextViews) is not desirable at all.

The thing is I was thinking about this case some time and I came up with a solution that uses a RecyclerView and does not introduce major changes to the core artifact (which stays the same). You can look here (it's in feature/plugins branch that I'm working on for the next major version 3.0.0). This branch is not stable yet (I'm stabilizing API and working on docs and tests). Anyway, one can look at how the adapter implementation is done here. It takes full Node and extracts all root Blocks. Then renders each block independently in a RecyclerView.Adapter. Please note I would not advice to use this branch as things are still moving around. But it can be used as a reference. As this thing can easily (well, I think it should be) be ported to current implementation (2.0.1).

I think it's better and more future-proof solution. The only downside is: there is no good TableLayout implementation on Android that would answer the most basic usage of markdown. Native android.widget.TableLayout is a mess. For example, if we wrap it in a HorizontalScrollView with a fillViewPort=true then TableLayout won't have line breaks (and in general can eat portions of content, which is unacceptable). There are different solutions based on RecyclerView, but they somehow overloaded with features that we won't need. Anyway, I was not looking into it very closely. Maybe I had missed a valid option. Please mention it here.

So, taking this into account, maybe, when used the solution I have just linked, only temporary we can render table in a WebView. But, as far as I remember, we won't be able to do real wrap_content for a WebView. So, this might not work at all.

if someone is interested I can later share my thoughts on a View that could display markdown tables (that would answer our needs). I anyway was doing to create an issue with help wanted label, but a bit later, when next release API will be stable to a certain degree.

@brucetoo
Copy link

brucetoo commented Jan 7, 2019

@noties Thanks for your so detailed comments. As you mentioned above, there are two key point:
1、Performance of multiple WebView in ScrollView

When there are so many webviews render in ScrollView once immediately, i admit it would be a performance problem. But according to your comments, How about replace ScrollView with RecyclerView ,let it handle recycler things. As far as i know, load html string(only include so simple table or whatever tags not supported or not easy to support by TextView) by local (webView.loadDataWithBaseURL) with WebViewmaybe better than do full html page from remote. That's why i advocate use WebView to do so, not only the images insert into table, but also the complicated css styles, i don't find a good way to perfect duplicate it with common View in Android.

2、WebView‘s real wrap_content property

About this point, as i mentioned in first comment: Let the webview ATMOST measured, that's maybe what you refer. you can check the custom webview that override onMeasure api here, which perfect make the content real wrap_content

Certainly, it's all be processing in my situation. (Just want to perfect duplicate rich-text styles html in android)

In this way, we maybe handle all the tags not perfect supported by TextView with some special Views. Like:
ImageView for <img>
VideoView for <video> etc....
Treating every tag as View Item in RecycerView

@noties
Copy link
Owner

noties commented Mar 18, 2019

Upcoming 3.0.0 release has a special artifact recycler-table that allows rendering a markdown table in a android's TableLayout. Still, if you wish to render it in a WebView it's still possible via regular recycler artifact by providing own TableBlock render entry

@noties
Copy link
Owner

noties commented May 2, 2020

There is support for images inside tables in upcoming 4.4.0-SNAPSHOT version

@noties noties reopened this May 2, 2020
@noties noties modified the milestones: 2.1.0, 4.4.0 May 2, 2020
@noties noties closed this as completed May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants