Initial idea for a table stream field block using handsontable.js.

[Proper-Job]

Hi,
This is an initial implementation of a StreamField table block using handsontable.js.
Let me know if there is anything else missing or gone totally astray.

Missing is documentation as to the available table configuration options.
If you like this initial approach I'll document it more clearly and completely.

No tests have been written yet.

[chrxr]

Hi @Proper-Job, thanks for this! It works very well in testing.

As opposed to implementing this in core, we'd propose refactoring this as a pip installable module. See this markdown module as a good example.

Could you implement in a similar way?

Cheers,
Chris

[gasman]

Hi @chrxr - I think the suggestion was for this to become a 'contrib' module within the Wagtail codebase (e.g. wagtail.contrib.table_block) rather than making it a completely separate pip-installable package.

[chrxr]

Hi @Proper-Job, we've discussed this in the team, and we'd like to go down the contrib route as suggested by @gasman above. Could you please update this PR. Thanks!

[Proper-Job]

Just came back from my holiday :)
Yes, I will refactor it into the the contrib module.
Thanks and greetings
Moritz

[Proper-Job]

I'm having a bit of a problem to integrate the git wagtail installation with PyCharm. Hence this ridiculous commit about ununsed imports.

[Proper-Job]

I squashed all of my commits. Is that alright to have all the logic in init.py, or do you want it in modules?

[gasman]

__init__.py is fine by me. Will give it a full review once the 1.2 release is out of the way - thanks!

[gasman]

Hi @Proper-Job - this is looking good! Just a few small (I hope) issues:

• It's possible to insert arbitrary HTML tags into cell text and have them interpreted as HTML. This needs to be changed so that any < > characters are displayed as-is in the output (i.e. the safe filters in templates/table_block/blocks/table.html should be removed, and the Handsontable behaviour changed so that it doesn't convert the tags, as in the examples at http://handsontable.com/examples.html).
• The table width in the editor is too wide - it pushes the 'delete' button in the block header off the edge of the window.
• Can we incorporate the JS from RichTextBlock so that the block height is automatically adjusted to fit the table height?

Once these are sorted, and we have tests and docs, I think we'll be ready to merge this. Thanks!

[seddonym]

Great addition!

One thing I noticed with this field is that it doesn't support tables with a vertical column too.

This could be done really simply by adding a 'First column header' checkbox and tweaking the template so it outputs for the first item in each row. Let me know if you'd like me to make a pull request for this.

[Proper-Job]

Sorry guys, for some reason I wasn't notified of your comments, for some reason that keeps happening to me. Let me review your comments and get back to you.

[Proper-Job]

@gasman It is possible to change HTML renderer to a simple text renderer for the table. All you would need for that is to pass the renderer at construction time like so:

table = TableBlock(table_options={'renderer':'text})

I could default it to text if you like. Should I do that?
I will check out the width problem.
I will try and incorporate the height js, too.

What kind of tests do you want to see for this?
Where do you want the docs to go?

[Proper-Job]

@seddonym Yes please do that.

[seddonym]

Hmm, I've tried to create a pull request, but I don't think Github supports multiple forking - I already have a fork of Wagtail in my account, so I can't fork from yours. I've manually forked it, but I'm not able to create a pull request back into your repo.

Probably easiest is if you merge my commits yourself from here:
https://github.com/seddonym/wagtail-tableblock/tree/streamfield-table

If you can think of another way to do it, let me know!

[Proper-Job]

@seddonym Thank you for that!
I merged your commits into this branch.

[bbusenius]

@Proper-Job and I have worked through the issues outlined by @gasman including unit tests. What's remaining is documentation. Can anyone tell me what's needed on the documentation front?

Edit: I can see there's a /wagtail/docs/reference/contrib/ folder. I imagine we would need to create a table_block.rst there. We've talked about documenting the table_options with links to relevant handsontable pages. Are there any other files we need to create or anything specific we need to document beyond the basics?

[bbusenius]

This is what we have for documentation at the moment. It's written in a hybrid rst format to make it easier to read on the web. We thought it would be a good idea to take comments before rolling it into the code base. If this is acceptable, we'll create a new table_block.rst in /wagtail/docs/reference/contrib/ and link it in docs/reference/contrib/index.rst. Please let us know if we're missing anything or if we should provide additional documentation files. This is just a rough draft. We'll go over it a few times before we commit.

[tomdyson]

Thanks, @bbusenius and @Proper-Job! I've made some minor tweaks in the Google doc but it's looking good so far. Perhaps you could add a screenshot demonstrating how table_block looks in the editor?

[bbusenius]

Thanks @tomdyson. A screenshot showing table_block in the editor is a great idea! We'll add one.

[bbusenius]

One thing that's missing that would be nice, is a sensible default icon. I don't see anything usable in the wagtail style guide but font awesome has this. What do you guys think?

[Proper-Job]

@gasman @tomdyson: All the issues outlined above have been addressed by @bbusenius and the PR was updated.
There is no default icon for the table block, yet. What do you think of @bbusenius suggestion above?
Would you like us to integrate a default icon?
If not, could you please review the changes?

[gasman]

Thanks! Will review shortly. The suggested icon looks good to me... (It seems we don't have any documentation for adding new icons to our font, but I believe it's done through https://icomoon.io/.)

[Proper-Job]

@gasman Just to double check. Would you like a new icon to be part of this PR? Or is that something you would like to address internally as there are no icon contributing guidelines?

[bbusenius]

I've been doing some preliminary research on the icons and my first impression is that this might best be handled by an internal wagtail design person. Using icomoon looks straightforward but how to integrate a new icon into a preexisting icon set is escaping me. From what I can tell, it looks like a set of icons would be generated locally and then uploaded to the project. I imagine the wagtail design team has a streamlined process for this. Please correct me if I am wrong.

[gasman]

OK, no worries - we'll handle it internally. (We've got another icon coming imminently in #2417, so we'd probably end up having to resolve the merge conflict anyhow :-) )

[gasman]

Merged in 9338fc2 + parents (without the icon - will handle that separately). Thanks all!

[alexgleason]

I needed to use this on Wagail 1.4, so I made a simple subtree split of the directory in master, restructured it, and released it as a pip installable package: wagtail-table-block.

https://github.com/alexgleason/wagtail_table_block

It says clearly in the readme to NOT use it if you can help it. But the database representation should be the same regardless of where it's coming from, so upgrading to Wagtail 1.5 should allow you to just delete the plugin and import from wagtail.contrib.

Just in case anyone else needs this.

[alexgleason]

Question: this uses from wagtail.contrib.table_block.fields import TableBlock

Do we need that .fields? Should it just be from wagtail.contrib.table_block import TableBlock?

Edit: In my own work, I'm using .blocks. See https://github.com/alexgleason/wagtailblocks_cards

But it might be better design to import it into __init__?

[gasman]

@alexgleason Looks like this was moved out of __init__ in 2d818af, in order to avoid problems on startup. I don't think the module naming is a big deal really.

[bbusenius]

There's an an issue with some of the logic being in __init__.py in Django 1.9. It throws an AppRegistryNotReady exception. The easiest way we could find to fix it was to move the code into a module.

[Proper-Job]

We chose to name the module fields.py because in the future the might be a TableField in addition to the TableBlock. In the absence of TableField it might be prudent to name the module blocks.py. I don't really mind either way.