Problem/Motivation

In the views page display there is a menu creation option. For the Normal Menu Entry, there is no expanded field. Which means you can only set the Menu Item's expanded status via the menu item page. Which works until you run cache-rebuild at which point it is reset to FALSE.

Proposed resolution

Patch attached, adds expanded field to the views UI/yml settings files. Editing works both ways (ie View <-> Menu Item) and no data is lost on cache clearing.

Also a couple extra asserts added to test that expanded and description are properly being set.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#11 views_page_display_menu-2598488-11.patch4.36 KBNickDickinsonWilde
#11 views_page_display_menu-2598488-11-testonly.patch1.55 KBNickDickinsonWilde
#9 views_page_display_menu-2598488-9.patch4.36 KBNickDickinsonWilde
#5 views_page_display_menu-2598488-5.patch4.35 KBNickDickinsonWilde
#2 views_page_display_menu-2598488-2.patch3.82 KBNickDickinsonWilde
#2 views_page_display_menu-2598488-2-testonly.patch1.55 KBNickDickinsonWilde
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

NickWilde created an issue. See original summary.

NickDickinsonWilde’s picture

NickDickinsonWilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
CreditAttribution: NickDickinsonWilde commented
Issue summary: View changes
FileSize
views_page_display_menu-2598488-2-testonly.patch1.55 KB
views_page_display_menu-2598488-2.patch3.82 KB

Status: Needs review » Needs work

The last submitted patch, 2: views_page_display_menu-2598488-2.patch, failed testing.

The last submitted patch, 2: views_page_display_menu-2598488-2-testonly.patch, failed testing.

NickDickinsonWilde’s picture

NickDickinsonWilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
CreditAttribution: NickDickinsonWilde commented
FileSize
views_page_display_menu-2598488-5.patch4.35 KB

oops missed config. Test note: this only tests that it is initially set correctly, should I also do a test to confirm that it is properly changed (ie one of the problems discussed in this patch).

NickDickinsonWilde’s picture

NickDickinsonWilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
CreditAttribution: NickDickinsonWilde commented
Status: Needs work » Needs review

The last submitted patch, 2: views_page_display_menu-2598488-2-testonly.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: views_page_display_menu-2598488-5.patch, failed testing.

NickDickinsonWilde’s picture

NickDickinsonWilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
CreditAttribution: NickDickinsonWilde commented
Status: Needs work » Needs review
FileSize
views_page_display_menu-2598488-9.patch4.36 KB

hmmph! used some 0/1 rather than T/F so schema didn't like it for BOOL values.

Status: Needs review » Needs work

The last submitted patch, 9: views_page_display_menu-2598488-9.patch, failed testing.

NickDickinsonWilde’s picture

NickDickinsonWilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
CreditAttribution: NickDickinsonWilde commented
FileSize
views_page_display_menu-2598488-11-testonly.patch1.55 KB
views_page_display_menu-2598488-11.patch4.36 KB

seriously! too many uploads of the same patches! (in the test had a _> instead of a ->).

NickDickinsonWilde’s picture

NickDickinsonWilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
CreditAttribution: NickDickinsonWilde commented
Status: Needs work » Needs review

The last submitted patch, 11: views_page_display_menu-2598488-11-testonly.patch, failed testing.

NickDickinsonWilde’s picture

NickDickinsonWilde
He/Him/His
Primary language English
Location Victoria, BC (T'So-uke lands)
CreditAttribution: NickDickinsonWilde commented
Issue tags: +rc target triage
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/views/src/Tests/Plugin/DisplayPageTest.php
@@ -136,6 +136,8 @@ public function testMenuLinks() {
+    $this->assertEqual($menu_link->getDescription(), 'Sample description.');

+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_page_display_menu.yml
@@ -91,10 +91,11 @@ display:
-        description: ''
+        description: 'Sample description.'

Nice test coverage expension

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

This sounds like data loss for me and due to that critical

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Tag1 Consulting commented
Priority: Major » Critical
Status: Reviewed & tested by the community » Fixed
Issue tags: -rc target triage

Yes consistent data-loss triggered from the UI should be treated as critical.

Patch looks great to me, so committed/pushed to 8.0.x, thanks!

  • catch committed 6f4690d on 8.0.x 
    Issue #2598488 by NickWilde: Views Page display menu expanded option is...

The last submitted patch, 11: views_page_display_menu-2598488-11-testonly.patch, failed testing.

Status: Fixed » Closed (fixed)

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