Currently we have to do stuff like this:

  $form['email_title'] = array(
    '#type' => 'item', 
    '#title' => t('E-mails'),
  );
  $form['email'] = array(
    '#type' => 'vertical_tabs',
  );

...when it would be a lot nicer to be able to do it from within the vertical tabs element itself:

  $form['email'] = array(
    '#type' => 'vertical_tabs',
    '#title' => t('E-mails'),
    '#description' => t('A helpful description for the overall vertical tab element here.'),
  );
Files: 
CommentFileSizeAuthor
#27 drupal8.vertical-tabs-title.27.patch5.17 KBsun
PASSED: [[SimpleTest]]: [MySQL] 49,580 pass(es). View
#24 drupal8.vertical-tabs-title.24.patch5.13 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.vertical-tabs-title.24.patch. Unable to apply patch. See the log in the details link for more information. View
#11 drupal8.vertical-tabs-title.11.patch4.93 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.vertical-tabs-title.11.patch. Unable to apply patch. See the log in the details link for more information. View
#8 drupal8.vertical-tabs-title.8.patch5.05 KBsun
PASSED: [[SimpleTest]]: [MySQL] 48,761 pass(es). View
#6 drupal8.vertical-tabs-title.6.patch5.03 KBsun
FAILED: [[SimpleTest]]: [MySQL] 47,994 pass(es), 1 fail(s), and 0 exception(s). View
#2 1144716-vertical-tabs-theme.patch6.13 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 29,461 pass(es). View
#1 1144716-vertical-tabs-theme.patch3.93 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 29,467 pass(es). View

Comments

Dave Reid’s picture

Status: Active » Needs review
Issue tags: +vertical tabs
FileSize
3.93 KB
PASSED: [[SimpleTest]]: [MySQL] 29,467 pass(es). View
Dave Reid’s picture

FileSize
6.13 KB
PASSED: [[SimpleTest]]: [MySQL] 29,461 pass(es). View

Rest of core's vertical tab elements converted.

Dave Reid’s picture

Adding tags for needing usability and markup reviews.

sun’s picture

Status: Needs review » Needs work
+++ b/includes/form.inc
@@ -3526,12 +3526,66 @@ function form_process_vertical_tabs($element, &$form_state) {
-  $element = $variables['element'];
+  $element = &$variables['element'];

This tweak could use a small code comment.

+++ b/includes/form.inc
@@ -3526,12 +3526,66 @@ function form_process_vertical_tabs($element, &$form_state) {
+
+

Duplicate newline.

+++ b/includes/form.inc
@@ -3526,12 +3526,66 @@ function form_process_vertical_tabs($element, &$form_state) {
+  $element += array(
+    '#title_display' => 'before',
+  );
...
+  // If #title is not set, we don't display any label or required marker.
+  if (!isset($element['#title'])) {
+    $element['#title_display'] = 'none';
+  }

Let's move the additional munging of #title_display right below the initial default code.

+++ b/includes/form.inc
@@ -3526,12 +3526,66 @@ function form_process_vertical_tabs($element, &$form_state) {
+  // Add element's #type and #name as class to aid with JS/CSS selectors.
+  $element['#attributes']['class'][] = 'form-item';
+  $element['#attributes']['class'][] = 'form-type-vertical-tabs';
...
+  $output = '<div' . drupal_attributes($element['#attributes']) . '>' . "\n";
...
+  $prefix = '<div class="vertical-tabs-panes">';
+  $suffix = '</div>';
...
+  $output .= "</div>\n";

One wrapper too much. Let's simply add the 'vertical-tabs-panes' class to the outer wrapper. More than one container doesn't make much sense here.

+++ b/includes/form.inc
@@ -3526,12 +3526,66 @@ function form_process_vertical_tabs($element, &$form_state) {
+  // Add a class for disabled elements to facilitate cross-browser styling.
+  if (!empty($element['#attributes']['disabled'])) {
+    $element['#attributes']['class'][] = 'form-disabled';
+  }

Not sure whether this has any [valid] effect here. Can probably be removed.

+++ b/includes/form.inc
@@ -3526,12 +3526,66 @@ function form_process_vertical_tabs($element, &$form_state) {
+  $prefix = '<div class="vertical-tabs-panes">';
+  $suffix = '</div>';
...
+    case 'attribute':

If there's a meaning to 'attribute', then the only possible meaning I could see would be a 'title' attribute on the wrapping DIV.

+++ b/includes/form.inc
@@ -3526,12 +3526,66 @@ function form_process_vertical_tabs($element, &$form_state) {
+    case 'none':
...
+      // Output no label and no required marker, only the children.

Stale reference to "and no required marker".

Powered by Dreditor.

David_Rothstein’s picture

This has the "Needs usability testing" tag - what kind of testing does it need exactly?

sun’s picture

Assigned: Dave Reid » sun
Category: feature » task
Status: Needs work » Needs review
Issue tags: -Needs usability testing, -Needs markup review
FileSize
5.03 KB
FAILED: [[SimpleTest]]: [MySQL] 47,994 pass(es), 1 fail(s), and 0 exception(s). View

I just stumbled over this in a contrib module.

I'd suggest to keep this simple and just get the job done.

Status: Needs review » Needs work

The last submitted patch, drupal8.vertical-tabs-title.6.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
5.05 KB
PASSED: [[SimpleTest]]: [MySQL] 48,761 pass(es). View

Fixed that test failure — of course, the #pre_render needs to check whether any tabs are accessible/visible.

This patch should come back green and could be considered ready.

sun’s picture

Feedback, anyone?

sun’s picture

#8: drupal8.vertical-tabs-title.8.patch queued for re-testing.

sun’s picture

FileSize
4.93 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.vertical-tabs-title.11.patch. Unable to apply patch. See the log in the details link for more information. View

Re-rolled against HEAD.

Feedback, anyone?

sun’s picture

Issue tags: +API clean-up
sun’s picture

Anyone?

brantwynn’s picture

#11 applies cleanly to drupal-8.x-dev. I'm not finding any manual errors when viewing forms that contain vertical tabs around the dev site.

If patch is committed it would save ~3 lines for title assertions on vertical tabs.

sun’s picture

Was that an RTBC? ;)

brantwynn’s picture

Status: Needs review » Reviewed & tested by the community
sun’s picture

Issue tags: -vertical tabs, -API clean-up
sun’s picture

sun’s picture

Issue tags: +vertical tabs, +API clean-up
YesCT’s picture

Manual testing. Looks find on blocks (admin/structure/block/manage/system/help/configure), account settings (admin/config/people/accounts), filters (admin/config/content/formats/filtered_html)
Content types like articles don't seems to have titles on the vertical tabs to convert. Still rtbc to me.

patch still applies with offsets.

YesCT’s picture

Issue tags: -vertical tabs, -API clean-up
catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +vertical tabs, +API clean-up

The last submitted patch, drupal8.vertical-tabs-title.11.patch, failed testing.

sun’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
5.13 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.vertical-tabs-title.24.patch. Unable to apply patch. See the log in the details link for more information. View

Re-rolled against HEAD.

catch’s picture

Issue tags: -vertical tabs, -API clean-up

Status: Reviewed & tested by the community » Needs work
Issue tags: +vertical tabs, +API clean-up

The last submitted patch, drupal8.vertical-tabs-title.24.patch, failed testing.

sun’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
5.17 KB
PASSED: [[SimpleTest]]: [MySQL] 49,580 pass(es). View

Hah, playing the re-roll monkey — so lucky to break my own patches :)

webchick’s picture

Category: task » feature

This looks like a nice feature/clean-up. We can commit it once we're under thresholds again.

webchick’s picture

Category: feature » task
Status: Reviewed & tested by the community » Fixed

Hm. Actually I guess catch has tried to re-test this several times so guess he's cool to commit this.

Committed and pushed to 8.x.

webchick’s picture

Title: Allow vertical tabs elements to have #title and #description values » Change notice: Allow vertical tabs elements to have #title and #description values
Priority: Normal » Critical
Status: Fixed » Active
Issue tags: +Needs change record

Der. Needs a change notice.

sun’s picture

Title: Change notice: Allow vertical tabs elements to have #title and #description values » Allow vertical tabs elements to have #title and #description values
Priority: Critical » Normal
Status: Active » Fixed
Issue tags: -Needs change record

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