Add/147 table block (alternate vision) #850
Conversation
|
I moved the controls into a menu |
|
Added the alignment controls and matched the CSS of the existing table: |
|
While this approach would definitely be the faster way to implement all the table controls, I do like to try to keep Editable small and inline. I don't think it's hard or impossible to implement all the controls like that, it's just code that is now in the table plugin that would be in this block registration, or possibly some more abstracted behaviour. In #62 block selection looks very much like cell selection. |
I do not understand this criticism - I have not changed Editable? |
|
After #849 which added a bunch of tests for block parsing and serialization, this PR should probably be rebased against |
|
From the screenshot alone I think this should be merged in for sure. It's missing a fullwide alignment option but that's trivial to add and probably something we should do separately! @youknowriad can you take a quick look at the code so we can get this in? Thanks both! |
I copied the alignment options off the existing table. |
|
I tried out align "full", it seems a bit glitchy for tables: Notably the up/down arrows get pushed off the screen and the horizontal scrollbar is shown. |
|
I think it's easy to fix the hoz scrollbar, and add the fullwide. But also worth doing separately, it's something I think I can do. So not something that should hold up this PR. |
blocks/library/table2/index.js
Outdated
| } | ||
| }, | ||
|
|
||
| edit( { attributes, setAttributes, focus, setFocus } ) { |
There was a problem hiding this comment.
There's a new className prop we should add to the root element of the edit function (see other blocks).
There was a problem hiding this comment.
I have propagated this prop to the editable.
| @@ -0,0 +1,36 @@ | |||
| .editor-visual-editor__block[data-type="core/table2"] { | |||
There was a problem hiding this comment.
I think we should encourage the use of the new generated classNames for styling wp-block-table2
There was a problem hiding this comment.
Ok, I have moved the table styles so they are a child of wp-block-table-2. I can't move the other styles though because the div with wp-block-table-2 class is not the same div that is selected by .editor-visual-editor__block[data-type="core/table2"].
blocks/library/table2/table-block.js
Outdated
| />, | ||
| focus && ( | ||
| <BlockControls key="menu"> | ||
| <ToolbarMenu |
There was a problem hiding this comment.
So if I understand correctly, the purpose of this component is to display a toolbar with a dropdown button.
What if I want to display several dropdown buttons in the same toolbar?
I think instead we'd probably want to refactor this to be able to do something like:
<Toolbar>
<DropdownMenu />
<DropdownMenu />
</Toolbar>There was a problem hiding this comment.
I have changed it so it nests inside a Toolbar and renamed it to DropdownMenu.
|
This block works great, I left some small notes but if @jasmussen thinks we should get this in today's release we can merge and resolve this in a follow-up PR. |
|
Whether it goes in todays release or not is less important, but I think it would be good to merge before the PR gets any bigger, and then open smaller PRs to fix any remaining issues. |
|
I addressed the feedback and merged it. |
|
Hooray! Thanks for working on this James! |
|
One of the original stated reasons for using the TinyMCE table plugin was the ability to make headings - it's a crucial element for accessibility for this block to support the ability to define a heading. See #1470 |
|
@joedolson I will add table headings to my TODO list. It might require some changes to the table plugin to implement. |
|
Tables are now very, very usable. For the first time in WP visual editor. People tend to fill cells, rows then they realise they filled wrong one. Happens often. |
The purpose of the pull request is to:
TinyMCE already provides execCommands to add columns, rows, make headings, and merge cells - I think we should be using them to make the implementation easier and more robust.
Note I have used the icons from the TinyMCE inlite theme for the table operations. I imagine they will have to be replaced before this is considered for merging by a more official icon but I couldn't find anything appropriate.