Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix navigation menus after Tsubaki headstart #69008

Closed
1 of 2 tasks
daledupreez opened this issue Oct 12, 2022 · 10 comments
Closed
1 of 2 tasks

Fix navigation menus after Tsubaki headstart #69008

daledupreez opened this issue Oct 12, 2022 · 10 comments
Assignees

Comments

@daledupreez
Copy link
Contributor

daledupreez commented Oct 12, 2022

Details

While testing some changes in D89811-code, I ran Headstart for the Tsubaki theme and noticed that two navigation menus didn't seem correct after the Headstart process completed:

  1. The top nav bar only includes a link to Shop, while the demo site has links to Shop, Blog, and My account.

Screenshot 2022-10-12 at 23 29 55

2. The "Need a help?" section in the footer only includes a link to _Shop_, while the demo site has links to _FAQ_, _Refund and Returns Policy_, and _Contact us_.

Screenshot 2022-10-12 at 23 36 19

Also, I think we should simplify "Need a help?" to just "Need help?", though that's not technically related to the issue we need to address.

These issues block #67862.

Checklist

  • Fix the header navigation
  • Fix the help navigation in the footer

Related

No response

@mreishus
Copy link
Contributor

mreishus commented Oct 12, 2022

the nav post in the annotation looks like:

        {
            "post_type": "wp_navigation",
            "post_title": "Header navigation",
            "post_content": "<!-- wp:navigation-link {\"label\":\"Shop\",\"type\":\"page\",\"id\":20,\"url\":\"https:\/\/tsubakidemo.wpcomstaging.com\/shop\/\",\"kind\":\"post-type\",\"isTopLevelLink\":true}
 \/-->\n\n<!-- wp:navigation-link {\"label\":\"Blog\",\"type\":\"page\",\"id\":6,\"url\":\"https:\/\/tsubakidemo.wpcomstaging.com\/blog\/\",\"kind\":\"post-type\",\"isTopLevelLink\":true}
 \/-->\n\n<!-- wp:navigation-link {\"label\":\"My account\",\"type\":\"page\",\"id\":23,\"url\":\"https:\/\/tsubakidemo.wpcomstaging.com\/my-account\/\",\"kind\":\"post-type\",\"isTopLevelLink\":true}
 \/-->",
            "post_name": "header-navigation",
            "post_excerpt": "",
            "menu_order": 0,
            "post_status": "publish",
            "comment_status": "closed",
            "ping_status": "closed",
            "_starter_page_template": "",
            "hs_old_id": 45,
            "hs_post_parent": 0,
            "hs_taxonomies": [],
            "hs_custom_meta": "_hs_extra"
        }

parse_navigation_link_content() in wp-content/lib/headstart/class-headstart-setters.php looks at the content, verifying that each ID exists as a post inserted in the annotation

  • shop, id 20
    • there's a post with "hs_old_id": 20, above, so a match is found and this is inserted
  • blog, id 6
    • It can't find something in new_post_ids with id=6, so it skips
    • also: /blog on my newly headstarted tsubaki test simple site leads to a 404 instead of the blog view
  • my-account, id 23
    • It can't find something in new_post_ids with id=23, so it skips
    • This looks to be a page provided by woocommerce?

No fix ideas yet, just describing what's happening

@nelsonec87 nelsonec87 self-assigned this Oct 24, 2022
@nelsonec87
Copy link
Contributor

The headstart annotation was removed in D90578-code.
I'll fix (if needed) the new annotation when it's available.

The issues were:

  1. As Matthew noted above, parse_navigation_link_content() checks if the provided ids exist. A simple fix is to remove the ids from the post_content navigation-links.
  2. The "Blog" link was pointing to a page that doesn't exist, so we need to add the page to the annotation.

@mreishus
Copy link
Contributor

There's a new annotation available now

@nelsonec87
Copy link
Contributor

nelsonec87 commented Oct 25, 2022

The theme has different navigation items in the header and footer:

Header:
image

Footer:
image

It's possible to create a new "Menu" from headstart with wp_navigation post_type, but actually using it seems more complex.

The footer navigation block is in the Footer "Template Part" (wp_template_part type), which is not handled as a normal post.

We could create a post to override the Part, setting the correct Menu, but it seems that the headstart code doesn't create this kind of post from the annotation.

I believe we have two options:

  • Work on the headstart code to make it handle Template Part posts.
  • If it's not a blocker, we could leave the two navigation with the same items.

@mreishus
Copy link
Contributor

mreishus commented Oct 25, 2022

Added second menu to annotation, but the footer isn't assigned to it

I've added the second wp_navigation to the annotation. This does not fix the problem, but means the user is one click away from a correct solution:

After headstart is run, two different navigation menus exist, but both the header and footer are assigned to the "header navigation"
2022-10-25_13-22

Changing the footer navigation block to the other menu makes the site look like the demo
2022-10-25_13-22_1


Figuring out how assignments work

Currently, we don't have a way to set this for the user. The blocks being rendered here are navigation blocks that look like this:

<!-- wp:navigation {"layout":{"type":"flex","justifyContent":"left","orientation":"horizontal"},"style":{"typography":{"lineHeight":"1.1875","fontSize":"16px","fontStyle":"normal","fontWeight":"600"}}} /-->

There are many different ways to assign these, (see WordPress/gutenberg#38291 (comment) : "The Goal" bullet list ), but the primary one seems to be setting the ref attribute. <!-- wp:navigation ref="1234"....

The navigation menu in the footer is specified in the theme's patterns/footer.php. Part of preparing the theme is removing the ref attribute on the navigation tag (See: p1665463016671209-slack-C02M88KJ684 ).

If a user changes the menu assignment as in my second screenshot above, it's saved with a ref attribute:
2022-10-25_13-23

This looks to be saved in a post_type called wp_template_part:

$ wp post list --url=mycoolsite.wordp ress.com --post_type=wp_template_part
+-----+------------+-----------+---------------------+-------------+
| ID  | post_title | post_name | post_date           | post_status |
+-----+------------+-----------+---------------------+-------------+
| 328 | Footer     | footer    | 2022-10-25 13:22:46 | publish     |
+-----+------------+-----------+---------------------+-------------+

To fix, we would need:

  • The headstart annotation to contain the "fixed' footer with the ref=1234 assignment.
  • The headstart application code can write out the wp_template_part post.
    • It can also do ID transformations so it can point the ref=1234 to the correct wp_navigation entry from the annotation.

@nelsonec87
Copy link
Contributor

Tests with the updated headstart annotation

Tsubaki demo:
image

New site after headstart:
image

All links from the header navigation are working.

According to p1666788662869279/1666787101.387779-slack-C0Q664T29 we can leave the footer as is for now.

@iamtakashi
Copy link
Contributor

iamtakashi commented Oct 26, 2022

All links from the header navigation are working.

🎉 Thank you!!

According to p1666788662869279/1666787101.387779-slack-C0Q664T29 we can leave the footer as is for now.

Ok, understood!

@daledupreez
Copy link
Contributor Author

@nelsonec87, have the Headstart annotation updates been deployed yet? Could you provide a reference for the change if they have? Thanks!

@nelsonec87
Copy link
Contributor

nelsonec87 commented Oct 26, 2022

@mreishus already deployed the annotation with the latest fixes (adding new pages, navigation, etc).
My last tests were made using the annotation version from the repository 👍

D90670-code
D90604-code

@nelsonec87
Copy link
Contributor

The annotation has already been updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants