Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

Rudimentary table support #33

Merged
merged 1 commit into from
Feb 23, 2016

Conversation

seato
Copy link
Contributor

@seato seato commented Feb 23, 2016

This PR adds an imperfect workaround for adding table support by adding two spans: ClickableTableSpan and DrawTableLinkSpan.

These spans provide limited table support by reconstructing the HTML and passing it to an app-defined implementation of ClickTableSpan (which could possibly forward it to a WebView similar to what's shown in the example).

The DrawTableSpan does not need to be implemented, but we expose it so that applications can display text other than "[tap for table]", which is useful for localization.

This PR is imperfect because once we start keeping track of a table tag, we are only able to keep track of other tags which android doesn't handle by default, so the raw html passed to the ClickableTableSpan won't necessarily be correct.

@seato seato mentioned this pull request Feb 23, 2016
@seato
Copy link
Contributor Author

seato commented Feb 23, 2016

If this looks good to merge, then I'll add a commit to update the README to reflect table support.

@seato
Copy link
Contributor Author

seato commented Feb 23, 2016

Also if it looks good, I'll add the proper Apache headers. I just realized I left them off.

@dschuermann
Copy link
Member

Yes looks okay, I think that the usefulness is limited but I am willing to merge :)

Please add the license headers and README additions as you proposed.

@seato seato force-pushed the html-table-support branch from c5b676e to d77bf1f Compare February 23, 2016 17:50
@seato
Copy link
Contributor Author

seato commented Feb 23, 2016

@dschuermann Applied the changes we discussed; I also increased the version in the build and README to 1.4 but I didn't update the tag because I see it's at v1.4 so I'm not sure what kind of system we've got going on there.

@seato
Copy link
Contributor Author

seato commented Feb 23, 2016

@dschuermann What should happen if the HtmlTextView encounters a table but the developer doesn't implement a ClickableTableSpan?

ATM I provide a default implementation for DrawTableLinkSpan which means <table>...</table> gets replaced with [tap for table] (unless the dev provides their own implementation), but tapping the text does nothing.

There's two routes we could go about this:

  1. Provide a default implementation for ClickableTableSpan which crashes if the HtmlTextView comes across a table and the user/dev clicks the replacement text.
  2. Crash in HtmltextView.setHtmlFromString() which is pretty early so that it developers don't miss it.

I like the idea of crashing early so that devs don't get caught with their pants down, but I can also do what you do with image getters and just log an error. Thoughts?

@dschuermann
Copy link
Member

I think currently, every tag that is not supported is simply not displayed. I would propose we do the same for tables if no ClickableTableSpan has been given and simply omit the whole table, even not showing [tap for table].

…ter used by a ClickTableSpan. Applications wanting to take advantage of this capability need to implement a ClickTableSpan and do whatever they'd like with the raw HTML (i.e. pass it into a webview).

In addition, a DrawTableSpan has been provided for applications that want to display text other than "[tap for table]", which should be useful for localization.
@seato seato force-pushed the html-table-support branch from d77bf1f to 7baea7b Compare February 23, 2016 22:17
@seato
Copy link
Contributor Author

seato commented Feb 23, 2016

@dschuermann Updated the PR and the default behavior is to not render the tables.

dschuermann pushed a commit that referenced this pull request Feb 23, 2016
@dschuermann dschuermann merged commit 17972e2 into SufficientlySecure:master Feb 23, 2016
@dschuermann
Copy link
Member

thanks!

@dschuermann
Copy link
Member

and released to jcenter...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants