[wp-trac] [WordPress Trac] #49025: Twenty Twenty: Modal menu link with hash should scroll to that section on the page.

[WordPress Trac]

#49025: Twenty Twenty: Modal menu link with hash should scroll to that section on
the page.
---------------------------+------------------------------
 Reporter:  acosmin        |       Owner:  (none)
     Type:  defect (bug)   |      Status:  new
 Priority:  normal         |   Milestone:  Awaiting Review
Component:  Bundled Theme  |     Version:  5.3.1
 Severity:  normal         |  Resolution:
 Keywords:                 |     Focuses:  ui, javascript
---------------------------+------------------------------

Comment (by raQai):

 Okay, I made some rather big changes now because some things simply did
 not work as intended.

 1. as @bluewill  already pointed out, the scrolling back to the previous
 location after toggeling the modal menu messes around with some things
 that might be intended.

 **Why is this an issue?**
 e.g. use a sticky header by applying a fixed property to it and every time
 you close the navigation again, you will first be put at the top and
 scrolled to the previous location after.

 **Why does this happen?**
 Because at the current state, the toggle will force a `position: fixed` on
 the `html` element to allow scrolling within the modal navigation without
 changing location on the current page. I also see this as an issue because
 it will move the view port to the top of the page which will look weird
 when using a fixed header navigation.

 **How to fix this?**
 Instead of changing the position attribute of the page, we can simply
 apply `overflow:hidden`. That's how the overlays on Pinterest work for
 example. For now, this can simply be achieved using either js or simply by
 using the already by the JavaScript applied class
 {{{
 body.showing-modal {
     overflow: hidden;
 }
 }}}

 2. And I know this might be a corner case, but let's say you have multiple
 pages setup which contain anchors. Using a plain JavaScript based
 solution, you will heavily have to rely on the current window location to
 use the same format as the href of the link.

 **Why is this an issue?**
 Let's say you have page `bar` setup with a parent page `foo`. The actual
 url would be `<host>/foo/bar/`. A valid anchor within the navigation could
 be `bar#baz` if `bar` only exists once leading to possibly false negative
 matches when checking the anchor link based on the setup of the page.

 **Why does this happen?**
 Well, because WordPress handles such URLs gracefully in the background,
 which is nice.

 **How to fix this?**
 We will need check whether the menu item url corresponds to an existing
 page and simplify the url for the JavaScript. I did this using the
 `wp_nav_setup_menu_item` filter to avoid messing around with the core
 classes.
 {{{
 function anchor_nav_menu_items($menu){
         // contains # and does not start with it
         if (strpos($menu->url,'#')) {
       // check if this is a relative url
       // FIXME this condition needs some additional work because it relys
 on using the host properly
       //       www.domain.com/foo/bar would probably not match
 https://domain.com/foo/bar
                 if (strpos(get_site_url(), $menu->url) === false) {
                         $menu_site_url = get_site_url(null, $menu->url);
          $menu_site_id = url_to_postid($menu_site_url);
          // check if the current site is referenced in the menu item
                         if ($menu_site_id === get_the_id()) {
                                 $menu_site_url_explode = explode('#',
 $menu->url);
                                 $menu->url = '#' .
 end($menu_site_url_explode);
                         }
                 }
         }

         return $menu;
 }

 add_filter('wp_setup_nav_menu_item', 'anchor_nav_menu_items', 1);
 }}}

 3. Using the above changes, some functions in the JavaScript can be
 simplified/removed (see attached index.js.diff and index.js)

 I would create a PR for this, but I do not know where to actually put the
 php code since I am only used to hooks and filters, not the WordPress core
 libraries.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/49025#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform