Feedback from Coker

Author: thatblindgeye

packages/react-core/src/components/Button/Button.tsx

@@ -111,7 +111,7 @@ export interface ButtonProps extends Omit<React.HTMLProps<HTMLButtonElement>, 'r
   isCircle?: boolean;
   /** @beta Flag indicating the button is a docked variant button. For use in docked navigation. */
   isDocked?: boolean;
-  /** @beta Flag indicating the dock button should display text. Only applies when isDocked is true. */
+  /** @beta Flag indicating the docked button should display text. Only applies when isDocked is true. */
   isTextExpanded?: boolean;
   /** @hide Forwarded ref */
   innerRef?: React.Ref<any>;

packages/react-core/src/components/MenuToggle/MenuToggle.tsx

@@ -55,7 +55,7 @@ export interface MenuToggleProps
   isSettings?: boolean;
   /** @beta Flag indicating the menu toggle is a docked variant. For use in docked navigation. */
   isDocked?: boolean;
-  /** @beta Flag indicating the dock toggle should display text. Only applies when isDock is true. */
+  /** @beta Flag indicating the docked toggle should display text. Only applies when isDocked is true. */
   isTextExpanded?: boolean;
   /** Elements to display before the toggle button. When included, renders the menu toggle as a split button. */
   splitButtonItems?: React.ReactNode[];

packages/react-core/src/components/Page/Page.tsx

@@ -24,11 +24,13 @@ export interface PageProps extends React.HTMLProps<HTMLDivElement> {
   variant?: 'default' | 'docked';
   /** @beta Flag indicating the docked nav is expanded on mobile. Only applies when variant is docked. */
   isDockExpanded?: boolean;
-  /** @beta Flag indicating the docked nav should display text on desktop. Only applies when variant is docked. */
+  /** @beta Flag indicating the docked nav should display text on desktop. Only applies when variant is docked, and will handle
+   * setting isTextExpanded on individual isDocked components.
+   * */
   isDockTextExpanded?: boolean;
-  /** The horizontal masthead content (e.g. <Masthead />). When using the dock variant, this content will only render at mobile viewports. */
+  /** The horizontal masthead content (e.g. <Masthead />). When using the docked variant, this content will only render at mobile viewports. */
   masthead?: React.ReactNode;
-  /** @beta Content to render in the vertical dock when variant of dock is used. At mobile viewports, this content will be replaced with the content passed to masthead. */
+  /** @beta Content to render in the vertical dock when variant of docked is used. At mobile viewports, this content will be replaced with the content passed to masthead. */
   dockContent?: React.ReactNode;
   /** Sidebar component for a side nav, recommended to be a PageSidebar. If set to null, the page grid layout
    * will render without a sidebar.

packages/react-core/src/demos/examples/Nav/NavDockedNav.tsx

@@ -171,23 +171,26 @@ export const NavDockedNav: React.FunctionComponent = () => {
   const mobileToggleRef = useRef<HTMLButtonElement>(null);
   const dockedToggleRef = useRef<HTMLButtonElement>(null);
 
-  const onToggleDock = () => {
-    const willBeExpanded = !isDockExpanded;
-    setIsDockExpanded(willBeExpanded);
-    setIsDockTextExpanded(!isDockTextExpanded);
+  const onMobileToggle = () => {
+    setIsDockExpanded(!isDockExpanded);
 
-    // Shift focus between mobile and docked toggle buttons
     setTimeout(() => {
-      if (willBeExpanded) {
-        // Opening: focus the docked toggle button
-        dockedToggleRef.current?.focus();
-      } else {
-        // Closing: focus the mobile toggle button
-        mobileToggleRef.current?.focus();
-      }
+      dockedToggleRef.current?.focus();
     }, 200);
   };
 
+  const onToggleDock = () => {
+    if (isDockExpanded) {
+      setIsDockExpanded(!isDockExpanded);
+
+      setTimeout(() => {
+        mobileToggleRef.current?.focus();
+      }, 200);
+    } else {
+      setIsDockTextExpanded(!isDockTextExpanded);
+    }
+  };
+
   // Mobile masthead - shown on mobile viewports only, hidden on desktop
   const mobileMasthead = (
     <Masthead display={{ default: 'inline' }} id="mobile-masthead">
@@ -198,7 +201,7 @@ export const NavDockedNav: React.FunctionComponent = () => {
             variant="plain"
             aria-label="Global navigation"
             isSidebarOpen={isDockExpanded}
-            onSidebarToggle={onToggleDock}
+            onSidebarToggle={onMobileToggle}
             isHamburgerButton
           />
         </MastheadToggle>
@@ -244,7 +247,7 @@ export const NavDockedNav: React.FunctionComponent = () => {
         <Toolbar isVertical>
           <ToolbarContent>
             <ToolbarItem>
-              <Nav onSelect={onNavSelect} variant="docked" isTextExpanded={isDockTextExpanded} aria-label="Global">
+              <Nav onSelect={onNavSelect} variant="docked" aria-label="Global">
                 <NavList>
                   <NavItem
                     preventDefault
@@ -312,80 +315,38 @@ export const NavDockedNav: React.FunctionComponent = () => {
             >
               <ToolbarItem>
                 {isDockTextExpanded ? (
-                  <MenuToggle
-                    ref={appsRef}
-                    variant="plain"
-                    icon={<ThIcon />}
-                    isDocked
-                    isTextExpanded={isDockTextExpanded}
-                    aria-label="Applications"
-                  >
+                  <MenuToggle ref={appsRef} variant="plain" icon={<ThIcon />} isDocked aria-label="Applications">
                     Applications
                   </MenuToggle>
                 ) : (
                   <Tooltip aria="none" aria-live="off" triggerRef={appsRef} content="Applications">
-                    <MenuToggle
-                      ref={appsRef}
-                      variant="plain"
-                      icon={<ThIcon />}
-                      isDocked
-                      isTextExpanded={isDockTextExpanded}
-                      aria-label="Applications"
-                    >
+                    <MenuToggle ref={appsRef} variant="plain" icon={<ThIcon />} isDocked aria-label="Applications">
                       Applications
                     </MenuToggle>
                   </Tooltip>
                 )}
               </ToolbarItem>
               <ToolbarItem>
                 {isDockTextExpanded ? (
-                  <Button
-                    ref={settingsRef}
-                    aria-label="Settings"
-                    isSettings
-                    variant="plain"
-                    isDocked
-                    isTextExpanded={isDockTextExpanded}
-                  >
+                  <Button ref={settingsRef} aria-label="Settings" isSettings variant="plain" isDocked>
                     Settings
                   </Button>
                 ) : (
                   <Tooltip aria="none" aria-live="off" triggerRef={settingsRef} content="Settings">
-                    <Button
-                      ref={settingsRef}
-                      aria-label="Settings"
-                      isSettings
-                      variant="plain"
-                      isDocked
-                      isTextExpanded={isDockTextExpanded}
-                    >
+                    <Button ref={settingsRef} aria-label="Settings" isSettings variant="plain" isDocked>
                       Settings
                     </Button>
                   </Tooltip>
                 )}
               </ToolbarItem>
               <ToolbarItem>
                 {isDockTextExpanded ? (
-                  <MenuToggle
-                    ref={helpRef}
-                    variant="plain"
-                    icon={<QuestionCircleIcon />}
-                    isDocked
-                    isTextExpanded={isDockTextExpanded}
-                    aria-label="Help"
-                  >
+                  <MenuToggle ref={helpRef} variant="plain" icon={<QuestionCircleIcon />} isDocked aria-label="Help">
                     Help
                   </MenuToggle>
                 ) : (
                   <Tooltip aria="none" aria-live="off" triggerRef={helpRef} content="Help">
-                    <MenuToggle
-                      ref={helpRef}
-                      variant="plain"
-                      icon={<QuestionCircleIcon />}
-                      isDocked
-                      isTextExpanded={isDockTextExpanded}
-                      aria-label="Help"
-                    >
+                    <MenuToggle ref={helpRef} variant="plain" icon={<QuestionCircleIcon />} isDocked aria-label="Help">
                       Help
                     </MenuToggle>
                   </Tooltip>