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

refactor(v2): various dropdown improvements #3585

Merged
merged 8 commits into from
Oct 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/docusaurus-theme-classic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@
"prismjs": "^1.20.0",
"prop-types": "^15.7.2",
"react-router-dom": "^5.1.2",
"react-toggle": "^4.1.1",
"use-onclickoutside": "^0.3.1"
"react-toggle": "^4.1.1"
},
"devDependencies": {
"@docusaurus/module-type-aliases": "2.0.0-alpha.65",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,89 @@ describe('themeConfig', () => {
});
});

test('should allow possible types of navbar items', () => {
const config = {
navbar: {
items: [
// Doc link
{
type: 'doc',
position: 'left',
docId: 'intro',
label: 'Introduction',
activeSidebarClassName: 'custom-class',
},
// Regular link
{
to: '/guide/',
label: 'Guide',
position: 'left',
activeBaseRegex: '/guide/',
},
// Regular dropdown
{
label: 'Community',
position: 'right',
items: [
{
label: 'Facebook',
href: 'https://.facebook.com/',
target: '_self',
},
{
label: 'GitHub',
href: 'https://github.com/facebook/docusaurus',
className: 'github-link',
},
],
},
// Doc version dropdown
{
type: 'docsVersionDropdown',
position: 'right',
dropdownActiveClassDisabled: true,
dropdownItemsBefore: [
{
href:
'https://www.npmjs.com/package/docusaurus?activeTab=versions',
label: 'Versions on npm',
className: 'npm-styled',
target: '_self',
},
],
dropdownItemsAfter: [
{
to: '/versions',
label: 'All versions',
className: 'all_vers',
},
],
},
// External link with custom data attribute
{
href: 'https://github.com/facebook/docusaurus',
position: 'right',
className: 'header-github-link',
'aria-label': 'GitHub repository',
},
// Docs version
{
type: 'docsVersion',
position: 'left',
label: 'Current version',
},
],
},
};
expect(testValidateThemeConfig(config)).toEqual({
...DEFAULT_CONFIG,
navbar: {
...DEFAULT_CONFIG.navbar,
...config.navbar,
},
});
});

test('should allow empty alt tags for the logo image in the header', () => {
const altTagConfig = {
navbar: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@
* LICENSE file in the root directory of this source tree.
*/

import React, {useState} from 'react';
import React, {useState, useRef, useEffect} from 'react';
import clsx from 'clsx';
import Link from '@docusaurus/Link';
import useBaseUrl from '@docusaurus/useBaseUrl';
import {useLocation} from '@docusaurus/router';
import {isSamePath} from '../../utils';
import useOnClickOutside from 'use-onclickoutside';
import type {
NavLinkProps,
DesktopOrMobileNavBarItemProps,
Expand Down Expand Up @@ -67,21 +66,28 @@ function NavItemDesktop({
className,
...props
}: DesktopOrMobileNavBarItemProps) {
const dropDownRef = React.useRef<HTMLDivElement>(null);
const dropDownMenuRef = React.useRef<HTMLUListElement>(null);
const [showDropDown, setShowDropDown] = useState(false);
useOnClickOutside(dropDownRef, () => toggle(false));
function toggle(state: boolean) {
if (state) {
const firstNavLinkOfULElement =
dropDownMenuRef?.current?.firstChild?.firstChild;

if (firstNavLinkOfULElement) {
(firstNavLinkOfULElement as HTMLElement).focus();
const dropdownRef = useRef<HTMLDivElement>(null);
const dropdownMenuRef = useRef<HTMLUListElement>(null);
const [showDropdown, setShowDropdown] = useState(false);

useEffect(() => {
const handleClickOutside = (event) => {
if (!dropdownRef.current || dropdownRef.current.contains(event.target)) {
return;
}
}
setShowDropDown(state);
}

setShowDropdown(false);
};

document.addEventListener('mousedown', handleClickOutside);
document.addEventListener('touchstart', handleClickOutside);

return () => {
document.removeEventListener('mousedown', handleClickOutside);
document.removeEventListener('touchstart', handleClickOutside);
};
}, [dropdownRef]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't like much making the code a bit more complex, but as it should reduce a bit the bundle size, let's do this.


const navLinkClassNames = (extraClassName?: string, isDropdownItem = false) =>
clsx(
{
Expand All @@ -97,11 +103,11 @@ function NavItemDesktop({

return (
<div
ref={dropDownRef}
ref={dropdownRef}
className={clsx('navbar__item', 'dropdown', 'dropdown--hoverable', {
'dropdown--left': position === 'left',
'dropdown--right': position === 'right',
'dropdown--show': showDropDown,
'dropdown--show': showDropdown,
})}>
<NavLink
className={navLinkClassNames(className)}
Expand All @@ -110,27 +116,27 @@ function NavItemDesktop({
onKeyDown={(e) => {
if (e.key === 'Enter') {
e.preventDefault();
toggle(true);
setShowDropdown(!showDropdown);
}
}}>
{props.label}
</NavLink>
<ul ref={dropDownMenuRef} className="dropdown__menu">
<ul ref={dropdownMenuRef} className="dropdown__menu">
{items.map(({className: childItemClassName, ...childItemProps}, i) => (
<li key={i}>
<NavLink
onKeyDown={(e) => {
if (i === items.length - 1 && e.key === 'Tab') {
e.preventDefault();
toggle(false);

const nextNavbarItem =
dropDownRef.current &&
(dropDownRef.current as HTMLElement).nextElementSibling;

setShowDropdown(false);

const nextNavbarItem = (dropdownRef.current as HTMLElement)
.nextElementSibling;

if (nextNavbarItem) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wonder if it is possible to shorten this code down to one-line?

 const nextNavbarItem = (dropdownRef.current as HTMLElement).nextElementSibling;

if (nextNavbarItem) {
  (nextNavbarItem as HTMLElement).focus();
}

I have tried using optional chaining, but I ran into the type error.
Any clue? @slorber

Copy link
Collaborator

Choose a reason for hiding this comment

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

Afaik it's annoying but we can't do anything about the types not being the proper ones.

The shortest is probably:

((dropDownRef.current as HTMLElement)
                    ?.nextElementSibling as HTMLElement)?.focus();

Not much better

(nextNavbarItem as HTMLElement).focus();
}
}
}
}}
activeClassName="dropdown__link--active"
Expand All @@ -146,16 +152,15 @@ function NavItemDesktop({

function NavItemMobile({
items,
position: _position,
className,
position: _position, // Need to destructure position from props so that it doesn't get passed on.
...props
}: DesktopOrMobileNavBarItemProps) {
const {pathname} = useLocation();
const [collapsed, setCollapsed] = useState(
() => !items?.some((item) => isSamePath(item.to, pathname)) ?? true,
);

// Need to destructure position from props so that it doesn't get passed on.
const navLinkClassNames = (extraClassName?: string, isSubList = false) =>
clsx(
'menu__link',
Expand Down
25 changes: 14 additions & 11 deletions packages/docusaurus-theme-classic/src/validateThemeConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,26 @@ exports.DEFAULT_CONFIG = DEFAULT_CONFIG;

const NavbarItemPosition = Joi.string().equal('left', 'right').default('left');

// TODO we should probably create a custom navbar item type "dropdown"
// having this recursive structure is bad because we only support 2 levels
// + parent/child don't have exactly the same props
const DefaultNavbarItemSchema = Joi.object({
items: Joi.array().optional().items(Joi.link('...')),
const BaseNavbarItemSchema = Joi.object({
to: Joi.string(),
href: URISchema,
label: Joi.string(),
position: NavbarItemPosition,
activeBasePath: Joi.string(),
activeBaseRegex: Joi.string(),
className: Joi.string(),
'aria-label': Joi.string(),
prependBaseUrlToHref: Joi.string(),
})
// We allow any unknown attributes on the links
// (users may need additional attributes like target, aria-role, data-customAttribute...)
.unknown();

// TODO we should probably create a custom navbar item type "dropdown"
// having this recursive structure is bad because we only support 2 levels
// + parent/child don't have exactly the same props
const DefaultNavbarItemSchema = BaseNavbarItemSchema.append({
items: Joi.array().optional().items(BaseNavbarItemSchema),
position: NavbarItemPosition,
activeBasePath: Joi.string(),
activeBaseRegex: Joi.string(),
});
// TODO the dropdown parent item can have no href/to
// should check should not apply to dropdown parent item
// .xor('href', 'to');
Expand All @@ -79,8 +82,8 @@ const DocsVersionDropdownNavbarItemSchema = Joi.object({
position: NavbarItemPosition,
docsPluginId: Joi.string(),
dropdownActiveClassDisabled: Joi.boolean(),
dropdownItemsBefore: Joi.array().items(DefaultNavbarItemSchema).default([]),
dropdownItemsAfter: Joi.array().items(DefaultNavbarItemSchema).default([]),
dropdownItemsBefore: Joi.array().items(BaseNavbarItemSchema).default([]),
dropdownItemsAfter: Joi.array().items(BaseNavbarItemSchema).default([]),
});

const DocItemSchema = Joi.object({
Expand Down
29 changes: 2 additions & 27 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4992,11 +4992,6 @@ archiver@^4.0.0:
tar-stream "^2.1.2"
zip-stream "^3.0.1"

are-passive-events-supported@^1.1.0:
version "1.1.1"
resolved "https://registry.yarnpkg.com/are-passive-events-supported/-/are-passive-events-supported-1.1.1.tgz#3db180a1753a2186a2de50a32cded3ac0979f5dc"
integrity sha512-5wnvlvB/dTbfrCvJ027Y4L4gW/6Mwoy1uFSavney0YO++GU+0e/flnjiBBwH+1kh7xNCgCOGvmJC3s32joYbww==

are-we-there-yet@~1.1.2:
version "1.1.5"
resolved "https://registry.yarnpkg.com/are-we-there-yet/-/are-we-there-yet-1.1.5.tgz#4b35c2944f062a8bfcda66410760350fe9ddfc21"
Expand Down Expand Up @@ -17689,7 +17684,7 @@ react-dev-utils@^9.1.0:
strip-ansi "5.2.0"
text-table "0.2.0"

react-dom@^16.10.2, react-dom@^16.8.4:
react-dom@^16.8.4:
version "16.13.1"
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.13.1.tgz#c1bd37331a0486c078ee54c4740720993b2e0e7f"
integrity sha512-81PIMmVLnCNLO/fFOQxdQkvEq/+Hfpv24XNJfpyZhTRfO0QcmQIF/PgCa1zCOj2w1hrn12MFLyaJ/G0+Mxtfag==
Expand Down Expand Up @@ -17869,7 +17864,7 @@ react-waypoint@^9.0.2:
prop-types "^15.0.0"
react-is "^16.6.3"

react@^16.10.2, react@^16.8.4:
react@^16.8.4:
version "16.13.1"
resolved "https://registry.yarnpkg.com/react/-/react-16.13.1.tgz#2e818822f1a9743122c063d6410d85c1e3afe48e"
integrity sha512-YMZQQq32xHLX0bz5Mnibv1/LHb3Sqzngu7xstSM+vrkE5Kzr9xE0yMByK5kMoTK30YVJE61WfbxIFFvfeDKT1w==
Expand Down Expand Up @@ -21175,26 +21170,6 @@ url@^0.11.0:
punycode "1.3.2"
querystring "0.2.0"

use-isomorphic-layout-effect@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/use-isomorphic-layout-effect/-/use-isomorphic-layout-effect-1.0.0.tgz#f56b4ed633e1c21cd9fc76fe249002a1c28989fb"
integrity sha512-JMwJ7Vd86NwAt1jH7q+OIozZSIxA4ND0fx6AsOe2q1H8ooBUp5aN6DvVCqZiIaYU6JaMRJGyR0FO7EBCIsb/Rg==

use-latest@^1.0.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/use-latest/-/use-latest-1.1.0.tgz#7bf9684555869c3f5f37e10d0884c8accf4d3aa6"
integrity sha512-gF04d0ZMV3AMB8Q7HtfkAWe+oq1tFXP6dZKwBHQF5nVXtGsh2oAYeeqma5ZzxtlpOcW8Ro/tLcfmEodjDeqtuw==
dependencies:
use-isomorphic-layout-effect "^1.0.0"

use-onclickoutside@^0.3.1:
version "0.3.1"
resolved "https://registry.yarnpkg.com/use-onclickoutside/-/use-onclickoutside-0.3.1.tgz#fdd723a6a499046b6bc761e4a03af432eee5917b"
integrity sha512-aahvbW5+G0XJfzj31FJeLsvc6qdKbzeTsQ8EtkHHq5qTg6bm/qkJeKLcgrpnYeHDDbd7uyhImLGdkbM9BRzOHQ==
dependencies:
are-passive-events-supported "^1.1.0"
use-latest "^1.0.0"

use@^3.1.0:
version "3.1.1"
resolved "https://registry.yarnpkg.com/use/-/use-3.1.1.tgz#d50c8cac79a19fbc20f2911f56eb973f4e10070f"
Expand Down