Popup improvements and acknowledgement of existing vite bug: vitejs/v…

…ite#3924
budlin2 · Nov 6, 2023 · f38543d · f38543d
1 parent 1ce4df9
commit f38543d
Show file tree

Hide file tree

Showing 10 changed files with 151 additions and 78 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -33,6 +33,7 @@
     "@storybook/testing-library": "^0.2.2",
     "@types/react": "^18.0.0",
     "@types/react-dom": "^18.0.0",
+    "@types/storybook__react": "^5.2.1",
     "@vitejs/plugin-react": "^1.3.0",
     "babel-loader": "^8.2.5",
     "storybook": "^7.5.2",

diff --git a/src/components/Popup/Item.tsx b/src/components/Popup/Item.tsx
@@ -0,0 +1,77 @@
+import React, { useState } from 'react';
+
+import { PopupItemT } from './PopupTypes';
+
+import _classes from './popup.module.css';
+import { classNames } from '../../utils';
+
+export interface ItemClassesT {
+    item?:          string,
+    itemHover?:     string,
+    itemActive?:    string,
+};
+
+export interface ItemStylesT {
+    item?:          React.CSSProperties,
+    itemHover?:     React.CSSProperties,
+    itemActive?:    React.CSSProperties,
+};
+
+interface Props {
+    classes?:       ItemClassesT,
+    styles?:        ItemStylesT,
+    item:           PopupItemT,
+};
+
+const Item = ({
+    classes,
+    styles,
+    item,
+}: Props) => {
+    //------------------------------------------------------------------------------------------------------------------
+    // State
+    //------------------------------------------------------------------------------------------------------------------
+    const [hovered, setHovered] = useState<boolean>(false);
+    const [active, setActive] = useState<boolean>(false);
+
+    //------------------------------------------------------------------------------------------------------------------
+    // Styling
+    //------------------------------------------------------------------------------------------------------------------
+    const itemClassName = classNames(
+        classNames(_classes?.item, classes?.item ),
+        hovered && classNames(_classes?.itemHover, classes?.itemHover),
+        active && classNames(_classes?.itemActive, classes?.itemActive),
+    );
+
+    console.log(itemClassName)
+
+    const itemStyle = {
+        ...styles?.item,
+        ...( hovered ? styles?.itemHover : {} ),
+        ...( active ? styles?.itemActive : {} ),
+    };
+
+    //------------------------------------------------------------------------------------------------------------------
+    // Event Handlers
+    //------------------------------------------------------------------------------------------------------------------
+    const handleMouseEnter = () => setHovered(true);
+    const handleMouseLeave = () => setHovered(false);
+    const handleMouseDown = () => setActive(true);
+    const handleMouseUp = () => setActive(false);
+
+    return (
+        <div key={ item.id }
+            className       ={ itemClassName }
+            style           ={ itemStyle }
+            onClick         ={ item.onClick }
+            onMouseEnter    ={ handleMouseEnter }
+            onMouseLeave    ={ handleMouseLeave }
+            onMouseDown     ={ handleMouseDown }
+            onMouseUp       ={ handleMouseUp }
+        >
+            { item.label }
+        </div>
+    );
+};
+
+export default Item;
diff --git a/src/components/Popup/PopupTypes.ts b/src/components/Popup/PopupTypes.ts
@@ -1,4 +1,4 @@
-import { ID } from "../Tuple/TupleTypes";
+import { ID } from '../Tuple/TupleTypes';
 
 export type HorizontalRuleT = 'hr';
 

diff --git a/src/components/Popup/index.tsx b/src/components/Popup/index.tsx
@@ -1,33 +1,27 @@
-import { CSSProperties, useEffect, useState } from 'react';
+import { CSSProperties, useEffect } from 'react';
 
 import { PopupItemsT } from './PopupTypes';
+import { classNames, getUniqueId } from '../../utils';
+import Item, { ItemClassesT, ItemStylesT } from './Item';
 
 import _classes from './popup.module.css';
-import { classNames } from '../../utils';
-import { ID } from '../Tuple/TupleTypes';
 
-export interface PopupStylesT {
+export interface PopupStylesT extends ItemStylesT {
     popup?:         CSSProperties,
-    itemHover?:     CSSProperties,
-    itemActive?:    CSSProperties,
-    item?:          CSSProperties,
     hr?:            CSSProperties,
 };
 
-export interface PopupClassesT {
+export interface PopupClassesT extends ItemClassesT {
     popup?:         string,
-    itemHover?:     string,
-    itemActive?:    string,
-    item?:          string,
     hr?:            string,
 };
 
 export interface Props {
-    position: { x: number, y: number },
-    items: PopupItemsT,
-    classes?: PopupClassesT,
-    styles?: PopupStylesT,
-    onClose?: () => void,
+    position:   { x: number, y: number },
+    items:      PopupItemsT,
+    classes?:   PopupClassesT,
+    styles?:    PopupStylesT,
+    onClose?:   () => void,
 }
 
 const Popup = ({
@@ -37,64 +31,44 @@ const Popup = ({
     styles,
     onClose,
 }: Props) => {
-    //------------------------------------------------------------------------------------------------------------------
-    // State
-    //------------------------------------------------------------------------------------------------------------------
-    const [hoveredId, setHoveredId] = useState<ID>(null);
-    const [activeId, setActiveId] = useState<ID>(null);
-
     //------------------------------------------------------------------------------------------------------------------
     // Styling
     //------------------------------------------------------------------------------------------------------------------
     const popupClassName = classNames(_classes?.popup, classes?.popup);
     const popupStyle: CSSProperties = { ...styles?.popup, left: x, top: y };
 
-    const itemClassNameBase = classNames(_classes?.item, classes?.item);
+    const itemClasses = {
+        item:       classes?.item,
+        itemHover:  classes?.itemHover,
+        itemActive: classes?.itemActive,
+    };
+
+    const itemStyles = {
+        item:       styles?.item,
+        itemHover:  styles?.itemHover,
+        itemActive: styles?.itemActive,
+    };
+
     const hrClassName = classNames(_classes?.hr, classes?.hr);
 
     //------------------------------------------------------------------------------------------------------------------
-    // Event Handlers
+    // Effects
     //------------------------------------------------------------------------------------------------------------------
-    const handleMouseEnter = (id: ID) => setHoveredId(id);
-    const handleMouseLeave = () => setHoveredId(null);
-    const handleMouseDown = (id: ID) => setActiveId(id);
-
     // TODO: WTF is this?
     useEffect(() => (() => onClose()), []);
 
     return (
         <div className={popupClassName} style={popupStyle}>
             { items.map( item => {
                 if (item == 'hr')
-                    return <hr className={ hrClassName } style={ styles?.hr } />;
-
-                const itemClassName = classNames(
-                    itemClassNameBase,
-                    hoveredId === item.id && _classes?.itemHover,
-                    hoveredId === item.id && classes?.itemHover,
-                    activeId === item.id && _classes?.itemActive,
-                    activeId === item.id && classes?.itemActive,
-                );
-
-                console.log(itemClassName)
-
-                const itemStyle = {
-                    ...styles?.item,
-                    ...( hoveredId === item.id ? styles?.itemHover : {} ),
-                    ...( activeId === item.id ? styles?.itemActive : {} ),
-                };
+                    return <hr key={ getUniqueId() } className={ hrClassName } style={ styles?.hr } />;
 
                 return (
-                    <div key={ item.id }
-                        className       ={ itemClassName }
-                        style           ={ itemStyle }
-                        onClick         ={ item.onClick }
-                        onMouseEnter    ={ () => handleMouseEnter(item.id) }
-                        onMouseLeave    ={ handleMouseLeave }
-                        onMouseDown     ={ () => handleMouseDown(item.id) }
-                    >
-                        { item.label }
-                    </div>
+                    <Item key={ item.id }
+                        classes ={ itemClasses }
+                        styles  ={ itemStyles }
+                        item    ={ item }
+                    />
                 );
             })}
         </div>

diff --git a/src/components/Popup/popup.module.css b/src/components/Popup/popup.module.css
@@ -14,7 +14,7 @@
     height: 1px;
     width: 100%;
     margin: 0;
-    background-color: black; /* line color */
+    background: black; /* line color */
 }
 
 .item {
@@ -26,12 +26,17 @@
     cursor: pointer;
 }
 
-.itemHover {
-    background: rgba(0, 0, 0, 0.5);
-    color: white;
-}
+/*
+    Not providing default styles for itemHover and itemActive:
+    This active issue details why:
+        https://github.com/vitejs/vite/issues/3924
+    Basically, Vite is screwing up the order by which CSS modules
+    are loaded, so the styles for itemHover and itemActive are
+    are being applied before the styles passed into them via
+    the `classes` prop.
+
+    It's weird. If I clear the cache, it will be correct on the first
+    page render and then incorrect on every subsequent page render.
 
-.itemActive {
-    background: black;
-    color: white;
-}
+    Keep an eye on styles that get passed in a production app.
+*/
diff --git a/src/components/Tuple/Tree/Leaf.tsx b/src/components/Tuple/Tree/Leaf.tsx
@@ -131,15 +131,9 @@ const Leaf = ({
     const leafClassName = classNames(
         _classes?.leaf_base,
         classes?.leaf_base,
-        isDraggedOver
-            ? classNames(_classes?.leaf_dragOver, classes?.leaf_dragOver)
-            : '',
-        hovering
-            ? classNames(_classes?.leaf_hover, classes?.leaf_hover)
-            : '',
-        renaming
-            ? classNames(_classes?.leaf_active, classes?.leaf_active)
-            : '',
+        isDraggedOver && classNames(_classes?.leaf_dragOver, classes?.leaf_dragOver),
+        hovering && classNames(_classes?.leaf_hover, classes?.leaf_hover),
+        renaming && classNames(_classes?.leaf_active, classes?.leaf_active),
     );
 
     const newNodeClassName = classNames(

diff --git a/src/components/Tuple/Tree/Tree.tsx b/src/components/Tuple/Tree/Tree.tsx
@@ -1,3 +1,5 @@
+import _classes from './tree.module.css';
+
 import { useContext, useEffect, useRef, useState } from 'react';
 
 import Root from './Root';
@@ -6,12 +8,12 @@ import { PopupDetailsT, TreeT } from './TreeTypes';
 import { ID, TupleContextT } from '../TupleTypes';
 import Trashcan from './Trashcan';
 import ScrollPane from '../../ScrollPane';
-import Popup, { PopupClassesT, PopupStylesT } from '../../Popup';
 import Branches from './Branches';
 
-import _classes from './tree.module.css';
 import { classNames } from '../../../utils';
 
+import Popup, { PopupClassesT, PopupStylesT } from '../../Popup';
+
 
 export interface TreeProps {
     tree:               TreeT,

diff --git a/src/components/Tuple/Tree/tree.module.css b/src/components/Tuple/Tree/tree.module.css
@@ -240,5 +240,5 @@
 
 .popupItem_hover {
     background: var(--COLOR-SECONDARY-3);
-    color: red;
+    color: var(--COLOR-PRIMARY-1);
 }
diff --git a/src/utils/index.ts b/src/utils/index.ts
@@ -42,4 +42,4 @@ const getRandomWord = () => {
 
 export const isObject = (obj: any) => typeof obj === 'object' && !Array.isArray(obj) && obj !== null;
 
-export const classNames = (...classNames: string[]): string => classNames.reduce((acc, cur) => cur ? (acc + ' ' + cur) : acc, '');
+export const classNames = (...classNames: string[]): string => classNames.reduce((acc, cur) => (cur ? (acc + cur) : acc) + ' ', '').trim();
Original file line number	Diff line number	Diff line change
Expand Up		@@ -42,4 +42,4 @@ const getRandomWord = () => {

		export const isObject = (obj: any) => typeof obj === 'object' && !Array.isArray(obj) && obj !== null;

		export const classNames = (...classNames: string[]): string => classNames.reduce((acc, cur) => cur ? (acc + ' ' + cur) : acc, '');
		export const classNames = (...classNames: string[]): string => classNames.reduce((acc, cur) => (cur ? (acc + cur) : acc) + ' ', '').trim();