Right now I have no idea of knowing if the verticle_tabs #default_tab actually works as describe.

Sadly there doesn't seem to be a test for testing this functionality. I have a feeling I'm doing something wrong which is why I can't for the life of me get the default_tab working but it might end up that this is just a bug.

Summary: Work has been done during sprint day on DrupalCon Amsterdam, adjusting paths since the structure of Drupal has changed the past 2 years.

Remaining tasks

  • Remove whitespace from patch.
  • Reroll patch.
CommentFileSizeAuthor
#12 1434670-12.patch2.64 KBAlPotapov
#11 1434670-11.patch2.64 KBAlPotapov
#8 1434670-8.patch2.66 KBAlPotapov
#7 1434670-7.patch2.59 KBAlPotapov
#2 1434670.patch2.28 KBbleen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ericduran’s picture

ericduran CreditAttribution: ericduran commented
Category: bug » task

Ok, I figured it out. The #default_tab needs to take in a id so in my case it was #default_tab = 'edit-mytabkey';

I still think this needs actual testing, at least to make sure the hidden value is on the page.

bleen’s picture

bleen CreditAttribution: bleen commented
Status: Active » Needs review
FileSize
1434670.patch2.28 KB

weird .. the docs are definitely wrong here: http://api.drupal.org/api/drupal/developer!topics!forms_api_reference.ht...

anyway, patch

bleen’s picture

bleen CreditAttribution: bleen commented

added new issue: #1534778: FAPI documentation is incorrect for #default_tab

alansaviolobo queued 2: 1434670.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2: 1434670.patch, failed testing.

alansaviolobo’s picture

alansaviolobo CreditAttribution: alansaviolobo commented
Issue summary: View changes
Issue tags: +Needs reroll
AlPotapov’s picture

AlPotapov CreditAttribution: AlPotapov commented
Issue tags: +Amsterdam2014
FileSize
1434670-7.patch2.59 KB

Rerolled the patch, but there is another test in the same category (FORM\ELEMENTSVERTICALTABSTEST) "Vertical tab wrappers are not displayed to unprivileged users." that is failing. I don't think this is related, but I'm going to take a look at it.

AlPotapov’s picture

AlPotapov CreditAttribution: AlPotapov commented
Status: Needs work » Needs review
FileSize
1434670-8.patch2.66 KB

Added missing "#access" element to form array, which caused one test to fail.
Now everything seems to be working fine.

bleen’s picture

bleen CreditAttribution: bleen commented
Status: Needs review » Needs work 
+++ b/core/modules/system/src/Tests/Form/ElementsVerticalTabsTest.php
@@ -58,4 +58,12 @@ function testWrapperNotShownWhenEmpty() {
+  ¶

+++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestVerticalTabsForm.php
@@ -23,29 +23,26 @@ public function getFormId() {
+    ¶
...
+    ¶
...
+        ¶

You have some white space issues ...

Thanks for taking the time to reroll this!! As you start to contribute more you should consider using Dreditor. Its a chrome/firefox plugin that helps with tons of things, but one usefull feature is that it highlights whitespace issues on uploaded patches

AdamEvertsson’s picture

AdamEvertsson CreditAttribution: AdamEvertsson commented
Issue summary: View changes
AlPotapov’s picture

AlPotapov CreditAttribution: AlPotapov commented
Status: Needs work » Needs review
FileSize
1434670-11.patch2.64 KB

bleen18, thank you for your comment! I have deleted the whitespaces, hope it works this time

AlPotapov’s picture

AlPotapov CreditAttribution: AlPotapov commented
FileSize
1434670-12.patch2.64 KB

Nope.

One more try.

The last submitted patch, 11: 1434670-11.patch, failed testing.

AlPotapov queued 11: 1434670-11.patch for re-testing.

bleen’s picture

bleen CreditAttribution: bleen commented

I think #12 looks great. Thanks AlPotapov ...

Lets let someone else mark this as RTBC since I helped with the original patch. That said, RTBC from me...

AdamEvertsson’s picture

AdamEvertsson CreditAttribution: AdamEvertsson commented
Status: Needs review » Reviewed & tested by the community

Ran the test via Simplytest.me and it came back good, with the exception of error messages saying "Verbose message", but those have been there the whole time if I remember correctly. Apart from that, the rest of the test was good. I'll let you re-open the issue if the "Verbose message" is in the way of RTBC this issue.

The last submitted patch, 7: 1434670-7.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 6fa0c88 and pushed to 8.0.x. Thanks!

  • alexpott committed 6fa0c88 on 8.0.x 
    Issue #1434670 by AlPotapov, bleen18 | ericduran: Add test for...

Status: Fixed » Closed (fixed)

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