From d831d2fce8198fb814ea4d3d8c311db5c388d04c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 20 Jul 2021 12:10:22 +0200 Subject: [PATCH] Simplify "active menu" logic for section menus Fixes #8776 --- docs/content/en/functions/hasmenucurrent.md | 2 + docs/content/en/variables/menus.md | 3 + hugolib/menu_test.go | 116 +++++++++++++++++++- hugolib/page__menus.go | 1 - hugolib/site.go | 4 + navigation/menu.go | 15 +++ navigation/pagemenus.go | 44 +++----- 7 files changed, 151 insertions(+), 34 deletions(-) diff --git a/docs/content/en/functions/hasmenucurrent.md b/docs/content/en/functions/hasmenucurrent.md index c7b8eb7a9..c53c91f94 100644 --- a/docs/content/en/functions/hasmenucurrent.md +++ b/docs/content/en/functions/hasmenucurrent.md @@ -24,4 +24,6 @@ aliases: [] returns `true` if the PAGE is the same object as the `.Page` in one of the **children menu entries** under MENUENTRY in a given MENU. +{{< new-in "0.86.0" >}} If MENUENTRY's `.Page` is a [section](/content-management/sections/) then, from Hugo `0.86.0`, this method also returns true for any descendant of that section.. + You can find its example use in [menu templates](/templates/menu-templates/). diff --git a/docs/content/en/variables/menus.md b/docs/content/en/variables/menus.md index d84837a43..9b8fe4d49 100644 --- a/docs/content/en/variables/menus.md +++ b/docs/content/en/variables/menus.md @@ -40,6 +40,9 @@ Reference to the [page object][page-object] associated with the menu entry. This will be non-nil if the menu entry is set via a page's front-matter and not via the site config. +.PageRef {{< new-in "0.86.0" >}} +: _string_
Can be set if defined in site config and the menu entry refers to a Page. [site.GetPage](/functions/getpage/) will be used to do the page lookup. If this is set, you don't need to set the `URL`. + .Name : _string_
Name of the menu entry. The `name` key, if set for the menu entry, sets diff --git a/hugolib/menu_test.go b/hugolib/menu_test.go index c43878090..a647c5bfa 100644 --- a/hugolib/menu_test.go +++ b/hugolib/menu_test.go @@ -33,7 +33,7 @@ menu: ` ) -func TestSectionPagesMenu(t *testing.T) { +func TestMenusSectionPagesMenu(t *testing.T) { t.Parallel() siteConfig := ` @@ -106,7 +106,7 @@ Menu Main: {{ partial "menu.html" (dict "page" . "menu" "main") }}`, } // related issue #7594 -func TestMenuSort(t *testing.T) { +func TestMenusSort(t *testing.T) { b := newTestSitesBuilder(t).WithSimpleConfigFile() b.WithTemplatesAdded("index.html", ` @@ -193,7 +193,7 @@ menu: ) } -func TestMenuFrontMatter(t *testing.T) { +func TestMenusFrontMatter(t *testing.T) { b := newTestSitesBuilder(t).WithSimpleConfigFile() b.WithTemplatesAdded("index.html", ` @@ -243,7 +243,7 @@ menu: } // https://github.com/gohugoio/hugo/issues/5849 -func TestMenuPageMultipleOutputFormats(t *testing.T) { +func TestMenusPageMultipleOutputFormats(t *testing.T) { config := ` baseURL = "https://example.com" @@ -301,7 +301,7 @@ menu: "main" } // https://github.com/gohugoio/hugo/issues/5989 -func TestMenuPageSortByDate(t *testing.T) { +func TestMenusPageSortByDate(t *testing.T) { b := newTestSitesBuilder(t).WithSimpleConfigFile() b.WithContent("blog/a.md", ` @@ -399,3 +399,109 @@ key2: key2_config camelCase: camelCase_config `) } + +func TestMenusShadowMembers(t *testing.T) { + b := newTestSitesBuilder(t).WithConfigFile("toml", ` +[[menus.main]] +identifier = "contact" +pageRef = "contact" +title = "Contact Us" +url = "mailto:noreply@example.com" +weight = 1 +[[menus.main]] +pageRef = "/blog/post3" +title = "My Post 3" +url = "/blog/post3" + +`) + + commonTempl := ` +Main: {{ len .Site.Menus.main }} +{{ range .Site.Menus.main }} +{{ .Title }}|HasMenuCurrent: {{ $.HasMenuCurrent "main" . }}|Page: {{ .Page }} +{{ .Title }}|IsMenuCurrent: {{ $.IsMenuCurrent "main" . }}|Page: {{ .Page }} +{{ end }} +` + + b.WithTemplatesAdded("index.html", commonTempl) + b.WithTemplatesAdded("_default/single.html", commonTempl) + + b.WithContent("_index.md", ` +--- +title: "Home" +menu: + main: + weight: 10 +--- +`) + + b.WithContent("blog/_index.md", ` +--- +title: "Blog" +menu: + main: + weight: 20 +--- +`) + + b.WithContent("blog/post1.md", ` +--- +title: "My Post 1: With No Menu Defined" +--- +`) + + b.WithContent("blog/post2.md", ` +--- +title: "My Post 2: With Menu Defined" +menu: + main: + weight: 30 +--- +`) + + b.WithContent("blog/post3.md", ` +--- +title: "My Post 2: With No Menu Defined" +--- +`) + + b.WithContent("contact.md", ` +--- +title: "Contact: With No Menu Defined" +--- +`) + + b.Build(BuildCfg{}) + + b.AssertFileContent("public/index.html", ` +Main: 5 +Home|HasMenuCurrent: false|Page: Page(/_index.md) +Blog|HasMenuCurrent: false|Page: Page(/blog/_index.md) +My Post 2: With Menu Defined|HasMenuCurrent: false|Page: Page(/blog/post2.md) +My Post 3|HasMenuCurrent: false|Page: Page(/blog/post3.md) +Contact Us|HasMenuCurrent: false|Page: Page(/contact.md) +`) + + b.AssertFileContent("public/blog/post1/index.html", ` +Home|HasMenuCurrent: false|Page: Page(/_index.md) +Blog|HasMenuCurrent: true|Page: Page(/blog/_index.md) +`) + + b.AssertFileContent("public/blog/post2/index.html", ` +Home|HasMenuCurrent: false|Page: Page(/_index.md) +Blog|HasMenuCurrent: true|Page: Page(/blog/_index.md) +Blog|IsMenuCurrent: false|Page: Page(/blog/_index.md) +`) + + b.AssertFileContent("public/blog/post3/index.html", ` +Home|HasMenuCurrent: false|Page: Page(/_index.md) +Blog|HasMenuCurrent: true|Page: Page(/blog/_index.md) +`) + + b.AssertFileContent("public/contact/index.html", ` +Contact Us|HasMenuCurrent: false|Page: Page(/contact.md) +Contact Us|IsMenuCurrent: true|Page: Page(/contact.md) +Blog|HasMenuCurrent: false|Page: Page(/blog/_index.md) +Blog|IsMenuCurrent: false|Page: Page(/blog/_index.md) +`) +} diff --git a/hugolib/page__menus.go b/hugolib/page__menus.go index e64ffa2c9..49d392c2f 100644 --- a/hugolib/page__menus.go +++ b/hugolib/page__menus.go @@ -56,7 +56,6 @@ func (p *pageMenus) menus() navigation.PageMenus { func (p *pageMenus) init() { p.pmInit.Do(func() { p.q = navigation.NewMenuQueryProvider( - p.p.s.Info.sectionPagesMenu, p, p.p.s, p.p, diff --git a/hugolib/site.go b/hugolib/site.go index fe7305b91..e687710bf 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -1452,6 +1452,10 @@ func (s *Site) assembleMenus() { menuConfig := s.getMenusFromConfig() for name, menu := range menuConfig { for _, me := range menu { + if types.IsNil(me.Page) && me.PageRef != "" { + // Try to resolve the page. + me.Page, _ = s.getPageNew(nil, me.PageRef) + } flat[twoD{name, me.KeyName()}] = me } } diff --git a/navigation/menu.go b/navigation/menu.go index 3ef06f6a9..7c6a1ccc7 100644 --- a/navigation/menu.go +++ b/navigation/menu.go @@ -32,6 +32,7 @@ var smc = newMenuCache() type MenuEntry struct { ConfiguredURL string // The URL value from front matter / config. Page Page + PageRef string // The path to the page, only relevant for site config. Name string Menu string Identifier string @@ -63,6 +64,8 @@ type Page interface { Section() string Weight() int IsPage() bool + IsSection() bool + IsAncestor(other interface{}) (bool, error) Params() maps.Params } @@ -106,16 +109,28 @@ func (m *MenuEntry) IsEqual(inme *MenuEntry) bool { // IsSameResource returns whether the two menu entries points to the same // resource (URL). func (m *MenuEntry) IsSameResource(inme *MenuEntry) bool { + if m.isSamePage(inme.Page) { + return m.Page == inme.Page + } murl, inmeurl := m.URL(), inme.URL() return murl != "" && inmeurl != "" && murl == inmeurl } +func (m *MenuEntry) isSamePage(p Page) bool { + if !types.IsNil(m.Page) && !types.IsNil(p) { + return m.Page == p + } + return false +} + func (m *MenuEntry) MarshallMap(ime map[string]interface{}) { for k, v := range ime { loki := strings.ToLower(k) switch loki { case "url": m.ConfiguredURL = cast.ToString(v) + case "pageref": + m.PageRef = cast.ToString(v) case "weight": m.Weight = cast.ToInt(v) case "name": diff --git a/navigation/pagemenus.go b/navigation/pagemenus.go index 1dfa71255..f783e30cf 100644 --- a/navigation/pagemenus.go +++ b/navigation/pagemenus.go @@ -15,6 +15,7 @@ package navigation import ( "github.com/gohugoio/hugo/common/maps" + "github.com/gohugoio/hugo/common/types" "github.com/pkg/errors" "github.com/spf13/cast" @@ -97,31 +98,25 @@ func PageMenusFromPage(p Page) (PageMenus, error) { } func NewMenuQueryProvider( - setionPagesMenu string, pagem PageMenusGetter, sitem MenusGetter, p Page) MenuQueryProvider { return &pageMenus{ - p: p, - pagem: pagem, - sitem: sitem, - setionPagesMenu: setionPagesMenu, + p: p, + pagem: pagem, + sitem: sitem, } } type pageMenus struct { - pagem PageMenusGetter - sitem MenusGetter - setionPagesMenu string - p Page + pagem PageMenusGetter + sitem MenusGetter + p Page } func (pm *pageMenus) HasMenuCurrent(menuID string, me *MenuEntry) bool { - // page is labeled as "shadow-member" of the menu with the same identifier as the section - if pm.setionPagesMenu != "" { - section := pm.p.Section() - - if section != "" && pm.setionPagesMenu == menuID && section == me.Identifier { + if !types.IsNil(me.Page) && me.Page.IsSection() { + if ok, _ := me.Page.IsAncestor(pm.p); ok { return true } } @@ -143,18 +138,15 @@ func (pm *pageMenus) HasMenuCurrent(menuID string, me *MenuEntry) bool { } } - if pm.p == nil || pm.p.IsPage() { + if pm.p == nil { return false } - // The following logic is kept from back when Hugo had both Page and Node types. - // TODO(bep) consolidate / clean - nme := MenuEntry{Page: pm.p, Name: pm.p.LinkTitle()} - for _, child := range me.Children { - if nme.IsSameResource(child) { + if child.isSamePage(pm.p) { return true } + if pm.HasMenuCurrent(menuID, child) { return true } @@ -172,20 +164,16 @@ func (pm *pageMenus) IsMenuCurrent(menuID string, inme *MenuEntry) bool { } } - if pm.p == nil || pm.p.IsPage() { + if pm.p == nil { return false } - // The following logic is kept from back when Hugo had both Page and Node types. - // TODO(bep) consolidate / clean - me := MenuEntry{Page: pm.p, Name: pm.p.LinkTitle()} - - if !me.IsSameResource(inme) { + if !inme.isSamePage(pm.p) { return false } - // this resource may be included in several menus - // search for it to make sure that it is in the menu with the given menuId + // This resource may be included in several menus. + // Search for it to make sure that it is in the menu with the given menuId. if menu, ok := pm.sitem.Menus()[menuID]; ok { for _, menuEntry := range menu { if menuEntry.IsSameResource(inme) {