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

Editable site menu #196

Open
jbmoelker opened this issue Nov 1, 2024 · 5 comments
Open

Editable site menu #196

jbmoelker opened this issue Nov 1, 2024 · 5 comments

Comments

@jbmoelker
Copy link
Member

User stories

As a website visitor,
I want to have access to an app/site menu,
so that I get an overview of the site content/structure and can directly navigate to pages.

As a content editor,
I want to be able to edit/compose the app/site menu,
so that I can guide users to specific pages.

Requirements

  • The menu items should be nestable (see example below) to support more complex menus.
  • The menu items should support links to internal pages, external links, and non-link items that only serve to group sub items.
  • The menu should use clear landmark (<nav> with aria-label or aria-labelledby).
  • The menu should not require any library yet, but should be suitable to be used in a [Radix Navigation Menu[(https://www.radix-ui.com/primitives/docs/components/navigation-menu#navigation-menu) or similar in a project using Head Start.

Example menu (taking Head Start itself as an example):

  • demos
    • Text block
    • Image block
    • Video block
  • docs
    • Getting Started
    • Routing
      • Editable redirects
    • Accessibility
@jbmoelker
Copy link
Member Author

I made a quick PoC for this feature in a clone of the No Dodos website, since it relied on the new linked record routing already in place there. My findings:

Result in the browser:

image

Works well to have a modular content field for menuItems in a singleton app model (model was already in No Dodos website)

image

I used a menu item that supported nested items (referring to its own content block):

image

I used a single menu item for all 3 types of items, with a required title and optional (for grouping only item) link field. Where the link could be a record or an external link (newly introduced model):

image

I introduced the external link model so it would be just as easy to link to an external as an internal page. But that gives a bit of an odd behaviour the next time you want to select a record to link to as suddenly that external url is an option you can select:

image

I discussed this with @jurgenbelien and would now instead opt for 3 separate menu item blocks:

  • menu item - internal link
  • menu item - external link
  • menu item - group

This setup is open for extension and more suitable for a customisable setup that Head Start strives to be. And it allows for stricter requirements on each block:

  • internal link: link (or page) is always required. title can be optional (?) as it could default to page.title (assuming that exists). sub items are optional.
  • external link: link (or url) is always required. title is also required as we need a label to display. sub items are optional.
  • group item: title is required. sub items are required, as otherwise, it's just a text that doesn't navigate anywhere.

Now for example an easy extension could be that each group item has an image and a text so you can make things like this:

image

@jbmoelker
Copy link
Member Author

For the implementation above, these are some of the code highlights:

MenuItem.fragment.graphql:

#import '@lib/routing/PageRoute.fragment.graphql'

fragment MenuItem on MenuItemRecord {
  id
  title
  link {
    __typename
    ... on HomePageRecord {
      title
    }
    ... on PageRecord {
      title
      ...PageRoute
    }
    ... on ExternalLinkRecord {
      title: url
      url
    }
  }
}

AppMenu.astro:

---
import MenuItem from '@blocks/MenuItem/MenuItem.astro';

const { items } = Astro.props;
---

<nav aria-label="Site menu">
  <ul>
    {items.map((item) => <MenuItem item={item} />)}
  </ul>
</nav>

MenuItem.astro:

---
import type { MenuItemFragment, SiteLocale } from '@lib/types/datocms';
import { getLocale } from '@lib/i18n';
import { getHref } from '@lib/routing';

export type MenuItem = MenuItemFragment & {
  items: MenuItem[];
};
interface Props {
  item: MenuItem;
  level?: number;
}

const {
  item: { title, link, items },
  level = 1,
} = Astro.props;
const locale = getLocale() as SiteLocale;
---

<li class={`level-${level}`}>
  {
    link ? (
      link.__typename === 'ExternalLinkRecord' ? (
        <a href={link.url} target='_blank'>
          {title}
        </a>
      ) : (
        <a href={getHref({ locale, record: link })}>{title}</a>
      )
    ) : (
      title
    )
  }
  {
    items.length ? (
      <ul>
        {items.map((item) => (
          <Astro.self item={item} level={level + 1} />
        ))}
      </ul>
    ) : null
  }
</li>

usage in layout:

#import '@blocks/MenuItem/MenuItem.fragment.graphql'

query DefaultLayout($locale: SiteLocale!) {
  # ...
  app(locale: $locale) {
   # ...
    menuItems {
      ...MenuItem
      items {
        ...MenuItem
        items {
          ...MenuItem
          items {
            ...MenuItem
            items {
              ...MenuItem
            }
          }
        }
      }
    }
  }
}
<AppMenu items={data.app.menuItems} />

@jbmoelker
Copy link
Member Author

Reference implementation of responsive menu (no nesting) in No Dodos website:
https://github.com/No-Dodos/nododos-website/blob/main/src/components/AppMenu/AppMenu.astro

@jbmoelker
Copy link
Member Author

Should we implement this in 2 stages (separate PRs)?:

  1. Basic menu with internal link menu items only. So CMS models and components with minimal responsive behaviour (dialog on small viewports?)
  2. Extended menu with sub menu items. So add group and external link menu item, with corresponding UI for nesting (popover/disclosures/...).

Q: can ... link menu items have sub items or should only group items be able to have those? Maybe dependents on the desired UI and patterns?

@jbmoelker
Copy link
Member Author

See #234 for implementation of External Link that could be mostly reused for Menu Item External.

jbmoelker added a commit that referenced this issue Jan 7, 2025
# Changes

- Adds an `AppMenu` to the default layout header. The menu items are
editable via the CMS (new models included in migration). The menu has
basic responsive behaviour with a full and compact mode. See
`AppMenu/README.md` for details.
- Adds icons for the open and close menu buttons.
- Refactors inline logic to small components so they could be reused in
multiple places (`AppLogo`, `IconButton`, `SearchLink` and
`UnstyledList`).
- Moves components that should only be used in layouts to
`src/layouts/`, just like the new `AppMenu`.
- Adds / updates 7 small READMEs.

Only `AppMenu` is really new. All the other changes are assets (icons),
refactors and documentation. So the number of files changed may be
significant. But the actual changes are not that big.

Note: other types of menu items (like external links) and nested menu
items can be added in a separate PR so this one doesn't become any
bigger.

# Associated issue

Part of #196.

# How to test

1. Open preview link
2. Verify the header contains the new app menu with items from the CMS
3. Verify the menu items work
4. Verify that when the menu items no longer fit, the menu items are
hidden and a menu button appears.
5. Verify the menu button opens the menu in a dialog and you can operate
it (focus management, tab and esc work).
6. Change the menu in the CMS
7. Verify the new menu items are rendered (requires local dev server)

# Checklist

- [x] I have performed a self-review of my own code
- [x] I have made sure that my PR is easy to review (not too big,
includes comments) (only `AppMenu` is really new, other files are moved
around and many short `README`s are added)
- [x] I have made updated relevant documentation files (in project
README, docs/, etc)
- ~I have added a decision log entry if the change affects the
architecture or changes a significant technology~
- [x] I have notified a reviewer

<!-- Please strike through and check off all items that do not apply
(rather than removing them) -->
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

1 participant