Add "Table class" option to views table formatter UI

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

Tagging. for an issue summary update following the standard template please.

This feature request will require test coverage as well

Thanks.

@smustgrave I've updated the issue summary to follow the standard template.

I've made an initial conversion of @dieuwe #10 patch to a merge request.

It does appear the table style config schema needs to css class option added. The test I added was failing without it. Updated issue summary to reflect this.

Setting to "Needs Review". I added a functionality test to confirm functionality is working, and I also tested manually.

GREAT work @PCate! RTBC! Removing the solved tags!

Nice!

So this is one of those cases test-only feature doesn't work as it reverts the schema change. So applied locally and reverted the changes but kept the schema, so the test will run

Behat\Mink\Exception\ElementNotFoundException: Element matching xpath "//table[contains(concat(" ", @class, " "), " test-css-table-class1 test-css-table-class2 ")]" not found.

Which shows the test itself.

Believe this one is good. Only thing not 100% sure is if it will need a CR.

Just tested—working for me! This just made my day for reals.

Screenshots below.

The issue summary clearly explains the problem and the change.

This is changing the UI, adding tag. The latest before and after screenshots should be available from the issue summary to help reviewers and committers. I updated the IS to state that they are in #26. Testing was done in #25 and manual testing in #26.

I read the MR and have commented on the user interface text.

Thanks @quietone - you're right! Just did that, think you suggestion is good. I also added "Table" to the title to be even clearer about the context. Please review.

Believe feedback has been addressed here.

Does this need an update hook to add the empty class to any existing views that use the table style?

#30 is correct, we need a post update for that, otherwise there will be mis-matches when someone next resaves the views UI or a different update runs, marking needs work.

Is there any documentation on how to write config update hooks for views? My understanding is that it is different than regular update hooks?

Using existing views update hooks as examples I tried adding an update hook for the table CSS class schema change. I also added the update hook task to the list of remaining tasks.

So with addition of upgrade path will need a test for the that hook.

So with addition of upgrade path will need a test for the that hook.

I've added a db schema upgrade test.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.