Add "Table class" option to views table formatter UI

Created on 4 April 2019, almost 6 years ago
Updated 11 February 2023, about 2 years ago

Drupal 8 Follow-up of the Drupal 7 issue #1689472: add class to table in table format โ†’ , which was committed on 2019-04-04.

Like before in Drupal 7 the Drupal 8 views table formatter only allows to set a class on rows in the UI but not on the table itself, while that would be consistent to other formatters and helpful in several cases.

How can one add a class to the rendered table when a view is outputted as table?

In html-list format for example one can add a class to a wrapper OR/AND to the list. Is this possible to the table as well?

Thanks!

โœจ Feature request
Status

Needs work

Version

10.1 โœจ

Component
Viewsย  โ†’

Last updated about 1 hour ago

Created by

๐Ÿ‡ฉ๐Ÿ‡ชGermany Anybody Porta Westfalica

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    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.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

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

  • Merge request !8749Add CSS class field to views table options. โ†’ (Open) created by pcate
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

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

  • Pipeline finished with Canceled
    9 months ago
    Total: 367s
    #222302
  • Pipeline finished with Success
    9 months ago
    Total: 559s
    #222306
  • Pipeline finished with Canceled
    9 months ago
    Total: 511s
    #222783
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

    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.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • Pipeline finished with Success
    9 months ago
    Total: 473s
    #222791
  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

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

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Anybody Porta Westfalica

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

  • Status changed to RTBC 8 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    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.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States dalemoore

    Just testedโ€”working for me! This just made my day for reals.

    Screenshots below.

  • Status changed to Needs work 8 months ago
  • ๐Ÿ‡ณ๐Ÿ‡ฟNew Zealand quietone

    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.

  • Status changed to Needs review 8 months ago
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Anybody Porta Westfalica

    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.

  • Pipeline finished with Success
    8 months ago
    Total: 593s
    #247520
  • Status changed to RTBC 8 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Believe feedback has been addressed here.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom longwave UK

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

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

    #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.

  • Status changed to Needs work 5 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

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

  • Pipeline finished with Failed
    5 months ago
    Total: 161s
    #318852
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • Pipeline finished with Success
    5 months ago
    Total: 1267s
    #318858
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

    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.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

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

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • Pipeline finished with Success
    3 months ago
    Total: 574s
    #374317
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

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

    I've added a db schema upgrade test.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • 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.

  • Pipeline finished with Success
    3 months ago
    Total: 1260s
    #374492
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • Pipeline finished with Failed
    3 months ago
    Total: 405s
    #374618
  • Status changed to Needs review 3 months ago
  • 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.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sagarmohite0031

    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-

  • Pipeline finished with Success
    3 months ago
    Total: 641s
    #377888
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

    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.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

    @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.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sagarmohite0031

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

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

    @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.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

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

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

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

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

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

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom oily Greater London

    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'.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    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.

  • Pipeline finished with Success
    about 2 months ago
    Total: 484s
    #421088
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

    I updated the input description text.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Believe all feedback has been addressed

  • Status changed to Needs work about 1 month ago
  • 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.

  • Pipeline finished with Failed
    30 days ago
    Total: 113s
    #439221
  • Pipeline finished with Failed
    30 days ago
    Total: 569s
    #439225
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States pcate

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

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom longwave UK

    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.

Production build 0.71.5 2024