Allow to configure the h3 element for view style plugins

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.phpundefined
@@ -267,7 +267,7 @@ function element_wrapper_type($none_supported = FALSE, $default_empty = FALSE) {
+  static function get_elements() {

Might as well make it public

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/StylePluginBase.phpundefined
@@ -285,6 +287,20 @@ public function buildOptionsForm(&$form, &$form_state) {
+        '#options' => FieldPluginBase::get_elements(),

Hmm, this means no overridden version of this will be used

Also, if you're changing every instance of the method at once, maybe it'd be good to just rename to getElements() now

I think when this has test coverage this is good to go.

Do we have a follow up issue to add more (html5) tags? This could be a good novice issue, as it's just added some more tags the to array in the config file.

Working on fixes and tests for this.

oops, I think we need to actually print the new variable in the templates? :)

Added some tests too.

This is what I have, so if you have tests too, we can compare.

Ahh, ok. So what shall we do? Not allow this feature on tables?

I need to know where to find it first. It does sound like a good idea in theory, but not sure where to find the configs. Should I be able to find them here - admin/structure/views/view/frontpage/edit?

So this will be on the style settings e.g. table when you add a grouping field to group the results by. You will then see another select field that lets you choose your tag. Hopefully this helps :)

Ok, so there has to be a better way to describe this, but maybe...

Go to:
Format -> Settings -> Grouping field Nr.1

Then select:
Content Title

And you will have a select box pop up with a list of elements to give you a choice of elements:
Title HTML Element

I think it might be useful to indicate here that if you're allowing folks to choose different headings and no headings at all - Choose the HTML element to wrap around the grouping title of a view. - it might be useful to encourage folks to learn how to use headings properly.

I did a quick test in VoiceOver with Safari and it seems to work correctly.

It would be better if you also added the HTML5 hidden attribute as per:
http://www.paciellogroup.com/blog/2012/05/html5-accessibility-chops-hidd...

So rather than simply providing the style information via CSS:
<div class="form-item form-type-checkbox form-item-style-options-grouping-0-rendered" style="display: none;">

It would look a bit more like:
<div class="form-item form-type-checkbox form-item-style-options-grouping-0-rendered" style="display: none;" hidden>

That needs to switch along with the style information so that the semantic information is being passed along to AT.

I'm not a Views guy. It's great, but my team works with it much more than I do. I was just trying to get where you were describing.

I can open a new issue and link back here for reference.

I could do that. Captions are optional. But why would you? Captions have meaning while headings have a different meaning. Surely the goal should be to properly style captions in CSS?

A caption is the only way to establish semantic relationships between a heading and a table. I think I'd like to see a good argument to allow folks to change it over. On the other hand, if the defaults are set properly it's probably not going to be a common occurrence.

• http://webaim.org/techniques/tables/data#caption
• http://usability.com.au/2005/06/accessible-data-tables-2005/#identifying
• http://www.jimthatcher.com/webcourse9.htm#wc9.1
• http://dev.opera.com/articles/view/creating-accessible-data-tables/

#22: drupal-1880100-22.patch queued for re-testing.

This exposes a new option in the UI to change the H3 to another tag, but the default is unchanged, so I don't think this needs any accessibility review.
It now also has tests.

The code looks great, and works, so RTBC!

#30: drupal-1880100-30.patch queued for re-testing.

Should be fine but just checking.

#30: drupal-1880100-30.patch queued for re-testing.

Working on reroll

Rerolled to current head

Removed from patch

CONFLICT (modify/delete): core/modules/views/templates/views-view-unformatted.tpl.php deleted in HEAD
CONFLICT (modify/delete): core/modules/views/templates/views-view-table.tpl.php deleted in HEAD
CONFLICT (modify/delete): core/modules/views/templates/views-view-list.tpl.php deleted in HEAD
CONFLICT (modify/delete): core/modules/views/templates/views-view-grid.tpl.php deleted in HEAD

public function getElements() (HEAD) changed to public static function getHTMLElementTypes() (PATCH)

CONFLICT (content): Merge conflict in core/modules/views/lib/Drupal/views/Tests/Handler/FieldWebTest.php
CONFLICT (content): Merge conflict in core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php

Adding twig file changes to patch as per @dawehner and @Cottser

Added to patch per @dawehner and @Cottser on IRC:
<{{ title_element_type }}>{{ title }}</{{ title_element_type }}>

Added on my own:

  * - title: The title of this group of rows.
+ * - title_element_type: The HTML tag element for the title.
  * - rows: A list of rows. Each row contains a list of columns.

To:
core/modules/views/templates/views-view-unformatted.html.twig
core/modules/views/templates/views-view-table.html.twig
core/modules/views/templates/views-view-list.html.twig
core/modules/views/templates/views-view-grid.html.twig

changed status

Failing in StyleTest.php

Did manual testing and found tables not working as expected. All other styles working as expected.

The four failures in patch #41 are due to an incorrectly written test. The test in all of these patches only tests against the table style using $style['type'] = 'table'; in the foreach loop

I did a quick and dirty change to $style['type'] = $plugin; and only table fails. The test would also have to handle rows for grid and html_list styles. Test passes these with exceptions.

#46 miscounted - I did all 4 styles.

During my manual testing of the styles one by one, I did not get these exceptions with the 'default' style. It started with 'grid'. Of coarse 'table' failed and I did not run 'html_list' through test. I was thinking it was a miss-written test.

I will run all four one by one later today.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.phpundefined
@@ -270,7 +270,7 @@ public function elementWrapperType($none_supported = FALSE, $default_empty = FAL
+  public static function getHTMLElementTypes() {

Should we use Html instead of HTML? I can't remember the 'proper' rules on camel casing.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/StylePluginBase.phpundefined
@@ -299,6 +301,20 @@ public function buildOptionsForm(&$form, &$form_state) {
+        '#options' => FieldPluginBase::getHTMLElementTypes(),

Do you think we should move this static method somewhere else? not FieldPluginBase. Just a thought, otherwise this patch is looking good.

I think it living of StylePluginBase is better, using FieldPluginBase::stuff around town looks a bit weird. The rest of the changes look good.

#58: vdc-1880100-58.patch queued for re-testing.

Nice work! I think this is almost a bug =)

1. +++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php
   @@ -548,7 +528,7 @@ public function buildOptionsForm(&$form, &$form_state) {
   -      '#options' => $this->getElements(),
   +      '#options' => StylePluginBase::getHtmlElementTypes(),
   

   Why static? Could we call $this->view->style_plugin->getHtmlElementTypes()? Or make $this->getElements() a wrapper around it? Or...

2. +++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/StylePluginBase.php
   @@ -376,6 +392,27 @@ public function wizardSubmit(&$form, &$form_state, WizardInterface $wizard, &$di
   +   * Provide a list of elements valid for field HTML.
   +   *
   +   * This function can be overridden by fields that want more or fewer
   +   * elements available, though this seems like it would be an incredibly
   +   * rare occurence.
   +   */
   +  public static function getHtmlElementTypes() {
   

   Why not move this method to pluginbase? This docs needs tuning.

The method is only called by buildOptionsForm() and we could get those tests green by calling $this->view->getStyle()->getHtmlElementTypes() but this also means that the style plugin defines what which html elements the field plugin may use. I'm not sure we want that.

How about simply leaving FieldPluginBase::getElements() as is (yes, duplicate the code in style plugin base)? This way there is no coupling, only an api addition and it can still be overridden.

So #67 with that comment fix?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php
@@ -593,7 +573,7 @@ public function buildOptionsForm(&$form, &$form_state) {
+      '#options' => StylePluginBase::getHtmlElementTypes(FALSE),
...
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php
@@ -637,7 +617,7 @@ public function buildOptionsForm(&$form, &$form_state) {
+      '#options' => StylePluginBase::getHtmlElementTypes(FALSE),

StylePluginBase::getHtmlElementTypes()

Tested manually with a table. Options none and default does not seem to work: those are rendered as "<0>" and "<>" inside the <caption> element.

Just the reroll here.

See #79

I found this was needed on a project today, so am trying to pick this up and give it a shake.

First patch is an attempt at a reroll of #81 from May, all the way up through some significant versions.
Many of the files have been moved around to different places, but that was the least of it.

Issues still exist, but this is a faithful re-roll of #81 - with many lines re-applied manually due to nearby changes.

With the following two significant changes just to make it work:

1. Re-introduced the function getElements() into /core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php ... which was being removed here, as discussed around StylePluginBase::getHtmlElementTypes() above. The reason being that the FieldPluginBase.php implements FieldHandlerInterface which requires getElements() due to the abstract definition.
   I have not replaced the usages of it in the forms #options lines yet - as this is currently a no-fix reroll for changes. But I think that those lines should also go back to how they were - as it seems that getElements() is the way things have gone.
2. Added a line to core/modules/views/config/schema/views.data_types.schema.yml to reflect the new config option. This being absent was causing one of the other unit tests to fail now. With the message

   Drupal\Tests\views\Kernel\Plugin\DisplayKernelTest::testDefaultOptions
   Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for views.view.test_display_defaults with the following errors: views.view.test_display_defaults:display.default.display_options.style.options.title_element_type missing schema
   

   This test is found in core/modules/views/tests/src/Kernel/Plugin/DisplayKernelTest.php
   After inserting the schema, that test cleared.

I tried making an interdiff (failed), it was messier than the patch itself due (in large part at least) to all the file renaming.
Experimental manual testing has been promising but glitchy, and I'll review the actual issues next. I'm just pushing this up as a place to roll the next issues from.

This is 'needs work' still, but I'd like to bump it up to see if the testbot can assimilate it OK. (I've been getting an unrelated false positive in local tests today - so I'm half expecting to see a fail from Drupal\Tests\views\Kernel\Handler\FieldDateTest::testFieldDate - but let's see.)

Updated the test (that applies solely to this patch) to use current test methods, removed deprecated calls that meant that test could not even run.
Tests now run, but fail, as the patch functionality isn't working. Starting with failing tests is at least a solid start :-}

The test in question:
php ./core/scripts/run-tests.sh --verbose --class "Drupal\views\Tests\Plugin\StyleTest::testCustomElementTitle"

This issue appears to be a duplicate of https://www.drupal.org/project/drupal/issues/1383696, or the other way around. https://www.drupal.org/project/drupal/issues/1383696 has updated code that appears to work with 9.5.x.