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.

Hi,

Test status -Fail

Steps to reproduce -
Create a view with a table style.
Open the table style options modal in the Views UI
See that there is not a form field to add custom CSS classes to table element.

Issue is reproducible. But unable to apply patch. Can see an error when the patch is applied.
Please see the screenshot.

Hi,

Test status -Fail

Issue is reproducible.
MR applied successfully but add custom CSS field is still not there.

Steps to reproduce -
Create a view with a table style.
Open the table style options modal in the Views UI
See that there is not a form field to add custom CSS classes to table element.

Refer Before and after screenshots-

The test failure was with a unrelated core module. Merging in latest 11.x changes included a fix. Tests are now passing.

After apply the MR to Drupal 11 site install, be sure to run database updates (drush updb) and clear caches. After applying MR and updates, when you export configuration you should see the schema changes for any views that use a table style.

@sagarmohite0031 the new CSS class form field should be at the bottom of the table style options form modal. Your screenshot does not show this.

See @dalemoore screenshot in #26 as an example.

Hello @pcate,
Attaching Before and after updated screenshots-
Please check once

@sagarmohite0031 I apologize, the field input actually is under the row class input. It used to be at the bottom of the modal form, but I forgot it was moved.

I manually tested the MR again with both 10.4 and 11.1 and everything still appears to be working, including see the field input.

Think one of the last things needed will be a CR. Also before/after screenshots should be added to the summary for quick glance.

I added before/after screenshots to the issue summary and created a draft CR.

Fixed broken images in issue summary. For some reason the did not save previously.

Removed 'Needs change record' tag.

I edited the CR as some information was missing. Please refer to the revision log where I have detailed my changes.

Also need to add to it that there is a schema change so users may need to run 'drush updb'.

Left 1 comment on the MR. Think adding a small example would be nice as I don't know if the text I provide if the code is auto generating the selector (.) to the front.

Rest of the feedback appears to be addresses so believe this is 98% there.

I updated the input description text.

Believe all feedback has been addressed

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. 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.

I merge in the upstream changes from 11.x and fix the merge conflicts. Setting back to RTBC.

I read the MR and did a brief manual test and can't see any changes that are required.

Committed ea29660 and pushed to 11.x. Thanks!

Also published the change record.

Automatically closed - issue fixed for 2 weeks with no activity.

The post update / ViewsConfigUpdater method added here doesn't handle deprecations.

views_post_update_update_remember_role_empty and views_post_update_views_data_argument_plugin_id both do implement it so could be used for examples.

🐛 Add proper deprecation notices in config entity presave bc layers Active has more details. We can open a follow-up to implement that, or revert this and add it back with support, but one of those should happen before 11.2.0 so re-opening for now.

Think this is a release blocked then.

Opened 🐛 Add ViewsConfigUpdater deprecation support for update_table_css_class Active to address this lack of deprecation, also it's not being run from updateAll, so new module config is not getting updated either it looks like

Thanks! Moving this back to fixed.