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

[Enterprise Search] Add nested sub navigation to SideNavLinks #77685

Merged
merged 3 commits into from
Sep 17, 2020

Conversation

cee-chen
Copy link
Contributor

Summary

This adds the subNav prop to the SideNavLink component to support nested sub navigation. Example usage:

const subNav = (
  <>
    <SideNavLink to="/red">Red Fish</SideNavLink>
    <SideNavLink to="/blue">Blue Fish</SideNavLink>
  </>
);

const sideNav = (
  <SideNav product={APP_SEARCH_PLUGIN}>
    <SideNavLink to="/one" subNav={subNav}>One Fish</SideNavLink>
    <SideNavLink to="/two">Two Fish</SideNavLink>
  <SideNav>
);

Let me know if this works for you @scottybollinger!

Screencaps

Example from above:

Example App Search Engines subnav:

Checklist

- for nested navigation
@cee-chen cee-chen added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 16, 2020
@cee-chen cee-chen requested review from scottybollinger and a team September 16, 2020 22:32
@cee-chen
Copy link
Contributor Author

cee-chen commented Sep 16, 2020

Also in theory you can continually nest as many levels deep as you need to but you probably shouldn't lol. (link hitboxes & highlight areas will get wonky, they're currently only optimized for 1 sub-level)

wonky

also the markup is horrendous 😂

- while attempting to expand the hitbox area of nested links as much as possible
Copy link
Contributor

@scottybollinger scottybollinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM! Thanks so much for doing this!

@cee-chen
Copy link
Contributor Author

cee-chen commented Sep 16, 2020

@scottybollinger Ah I forgot to mention how to implement this with conditional display (i.e. only show the subnav when you're on the groups page). App Search does this a bit differently so I'm currently spitballing...

In some example new file (maybe components/layout/subnavs.tsx or maybe views/groups/subnav.tsx? Could go either way):

import { useParams } from 'react-router-dom';

export const groupsSubNav = () => {
  const { groupId } = useParams();
  if (!groupId) return null; // Or some other way of checking the route matches your groups route

  return (
     // nested SideNavLinks
  );
};

and then in layout/nav.tsx for workplace search:

import { groupsSubNav } from '../groups_view_folder/wherever';

// ...

      <SideNavLink to="/groups" subNav={groupsSubNav}>
        {i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.groups', {
          defaultMessage: 'Groups',
        })}
      </SideNavLink>

@cee-chen
Copy link
Contributor Author

Not sure what's up with that failing test, it's not ours 🤷

@elasticmachine merge upstream

@cee-chen
Copy link
Contributor Author

@elasticmachine merge upstream

@scottybollinger
Copy link
Contributor

@scottybollinger Ah I forgot to mention how to implement this with conditional display (i.e. only show the subnav when you're on the groups page). App Search does this a bit differently so I'm currently spitballing...

In some example new file (maybe components/layout/subnavs.tsx or maybe views/groups/subnav.tsx? Could go either way):

import { useParams } from 'react-router-dom';

export const groupsSubNav = () => {
  const { groupId } = useParams();
  if (!groupId) return null; // Or some other way of checking the route matches your groups route

  return (
     // nested SideNavLinks
  );
};

and then in layout/nav.tsx for workplace search:

import { groupsSubNav } from '../groups_view_folder/wherever';

// ...

      <SideNavLink to="/groups" subNav={groupsSubNav}>
        {i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.groups', {
          defaultMessage: 'Groups',
        })}
      </SideNavLink>

Thanks! What I'm doing is checking the logic file for a group.id, which is only set when the user is where they need to be to show the nav.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
enterpriseSearch 434.5KB +898.0B 433.7KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@cee-chen cee-chen merged commit 9d1ba4d into elastic:master Sep 17, 2020
@cee-chen cee-chen deleted the sub-navigation branch September 17, 2020 00:50
cee-chen pushed a commit that referenced this pull request Sep 17, 2020
#77694)

* Add subNav prop to SideNavLinks

- for nested navigation

* [Probably extra] Tweak CSS to allow for recursive subnesting

- while attempting to expand the hitbox area of nested links as much as possible

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants