diff --git a/v3/cypress/e2e/adornments.spec.ts b/v3/cypress/e2e/adornments.spec.ts index 41d7f784e2..b0397f4c30 100644 --- a/v3/cypress/e2e/adornments.spec.ts +++ b/v3/cypress/e2e/adornments.spec.ts @@ -461,7 +461,7 @@ context("Graph adornments", () => { // @ts-expect-error -- type definition is incorrect: return value is property value not chained element .should("have.css", "left").should((left: string) => { expect(left).to.include("px") - expect(parseInt(left, 10)).to.be.within(271, 273) + expect(parseInt(left, 10)).to.be.within(260, 275) }) // TODO: Test drag and drop of label and saving of dropped coordinates cy.get("[data-testid=adornment-checkbox-show-measure-labels]").click() diff --git a/v3/cypress/e2e/axis.spec.ts b/v3/cypress/e2e/axis.spec.ts index f17f703e7d..9819cf34a0 100644 --- a/v3/cypress/e2e/axis.spec.ts +++ b/v3/cypress/e2e/axis.spec.ts @@ -294,7 +294,8 @@ context("Test graph axes with various attribute types", () => { ah.verifyXAxisTickMarksDisplayed() ah.verifyYAxisTickMarksDisplayed() // With multiple y-attributes, each gets its own separate label - cy.get("[data-testid=graph]").find("[data-testid=attribute-label]").should("have.text", "LifeSpan") + cy.get("[data-testid=graph]").find(".axis-wrapper.bottom [data-testid=attribute-label]") + .should("have.text", "LifeSpan") cy.get("[data-testid=graph]").find("[data-testid=attribute-label-multi-y]").should("have.length", 2) cy.get("[data-testid=graph]").find("[data-testid=attribute-label-multi-y]").eq(0).should("have.text", "Height") cy.get("[data-testid=graph]").find("[data-testid=attribute-label-multi-y]").eq(1).should("have.text", "Sleep") @@ -309,9 +310,10 @@ context("Test graph axes with various attribute types", () => { toolbar.getUndoTool().click() cy.wait(500) // After undo, only one y-attribute remains, so it uses the standard attribute-label - cy.get("[data-testid=graph]") - .find("[data-testid=attribute-label]") - .should("have.text", "LifeSpanHeight") + cy.get("[data-testid=graph]").find(".axis-wrapper.bottom [data-testid=attribute-label]") + .should("have.text", "LifeSpan") + cy.get("[data-testid=graph]").find(".axis-wrapper.left [data-testid=attribute-label]") + .should("have.text", "Height") cy.get("[data-testid=graph]").find("[data-testid=attribute-label-multi-y]").should("have.length", 0) ah.verifyYAxisTickMarksDisplayed() ah.verifyAxisTickLabel("left", "0", 0) @@ -320,7 +322,8 @@ context("Test graph axes with various attribute types", () => { toolbar.getRedoTool().click() cy.wait(500) // After redo, multiple y-attributes restored, each gets its own label - cy.get("[data-testid=graph]").find("[data-testid=attribute-label]").should("have.text", "LifeSpan") + cy.get("[data-testid=graph]").find(".axis-wrapper.bottom [data-testid=attribute-label]") + .should("have.text", "LifeSpan") cy.get("[data-testid=graph]").find("[data-testid=attribute-label-multi-y]").should("have.length", 2) cy.get("[data-testid=graph]").find("[data-testid=attribute-label-multi-y]").eq(0).should("have.text", "Height") cy.get("[data-testid=graph]").find("[data-testid=attribute-label-multi-y]").eq(1).should("have.text", "Sleep") diff --git a/v3/cypress/e2e/graph.spec.ts b/v3/cypress/e2e/graph.spec.ts index 094f86a683..043892a6f0 100644 --- a/v3/cypress/e2e/graph.spec.ts +++ b/v3/cypress/e2e/graph.spec.ts @@ -765,7 +765,7 @@ context("Graph UI", () => { // TODO: See comment above regarding number of bars. // cy.get("[data-testid=bar-cover]").should("exist").and("have.length", 9) cy.get("[data-testid=bar-cover]").should("exist") - cy.get("[data-testid=axis-legend-attribute-button-top").click() + cy.get("[data-testid=axis-legend-attribute-button-top]").click() cy.get("[role=menuitem]").contains("Remove Side-by-side Layout by Diet").click() graph.getDisplayConfigButton().click() cy.get("[data-testid=bar-chart-checkbox]").click() diff --git a/v3/cypress/support/helpers/axis-helper.ts b/v3/cypress/support/helpers/axis-helper.ts index 3fa66972c3..70747373c9 100644 --- a/v3/cypress/support/helpers/axis-helper.ts +++ b/v3/cypress/support/helpers/axis-helper.ts @@ -2,7 +2,7 @@ import { AxisElements as ae } from "../elements/axis-elements" export const AxisHelper = { verifyDefaultAxisLabel(axis: string) { - ae.getDefaultAxisLabel(axis).should("have.text", "Click here, or drag an attribute here.") + ae.getDefaultAxisLabel(axis).should("have.text", "Drag an attribute or click here") }, verifyAxisLabel(axis: string, name: string) { ae.getAxisLabel(axis).should("have.text", name) diff --git a/v3/src/components/axis/axis-types.ts b/v3/src/components/axis/axis-types.ts index 18293bc39f..3985c585e4 100644 --- a/v3/src/components/axis/axis-types.ts +++ b/v3/src/components/axis/axis-types.ts @@ -2,6 +2,8 @@ import {axisBottom, axisLeft, axisRight, axisTop, ScaleBand, ScaleContinuousNumeric, ScaleOrdinal, select, Selection} from "d3" export const axisGap = 5 +export const labelMargin = 13 // whitespace outside the label background rect (between rect and axis bounds) +export const labelPaddingX = 8 // horizontal padding inside the label background rect (between rect edge and text) // "rightCat" and "top" can only be categorical axes. "rightNumeric" can only be numeric export const AxisPlaces = ["bottom", "left", "rightCat", "top", "rightNumeric"] as const diff --git a/v3/src/components/axis/axis-utils.ts b/v3/src/components/axis/axis-utils.ts index 3ea295b165..a4d0585bfb 100644 --- a/v3/src/components/axis/axis-utils.ts +++ b/v3/src/components/axis/axis-utils.ts @@ -1,4 +1,4 @@ -import {ScaleContinuousNumeric, ScaleLinear} from "d3" +import {BaseType, ScaleContinuousNumeric, ScaleLinear, Selection} from "d3" import { MutableRefObject } from "react" import { logMessageWithReplacement } from "../../lib/log-message" import { IDataConfigurationModel } from "../data-display/models/data-configuration-model" @@ -9,7 +9,7 @@ import { determineLevels } from "../../utilities/date-utils" import { GraphLayout } from "../graph/models/graph-layout" import { ITileModel } from "../../models/tiles/tile-model" import { kAxisGap, kAxisTickLength, kDefaultFontHeight } from "./axis-constants" -import {AxisPlace} from "./axis-types" +import {AxisPlace, labelPaddingX} from "./axis-types" import { updateAxisNotification } from "./models/axis-notifications" import { IBaseNumericAxisModel } from "./models/base-numeric-axis-model" @@ -356,3 +356,62 @@ export function zoomAxis( } ) } + +interface IRenderLabelBackgroundOptions { + gSelection: Selection + textSelector: string + transform?: string + visibility?: string +} + +/** + * Renders a background rect and dropdown arrow behind/after an axis or legend label text element. + * The rect provides hover/focus/selected visual states via CSS classes on the parent . + */ +export function renderLabelBackground( + { gSelection, textSelector, transform = '', visibility }: IRenderLabelBackgroundOptions +) { + const textNode = gSelection.select(textSelector).node() as SVGTextElement | null + const textBBox = textNode?.getBBox() + if (!textBBox) return + + const paddingX = labelPaddingX + const paddingY = 2 + const arrowWidth = 24 + + const rectWidth = textBBox.width + paddingX + arrowWidth + const rectHeight = textBBox.height + paddingY * 2 + const rectX = textBBox.x - paddingX + const rectY = textBBox.y - paddingY + + const rectSelection = gSelection.selectAll('rect.attribute-label-bg') + .data([1]) + .join((enter: any) => enter.append('rect').attr('class', 'attribute-label-bg')) + .attr('x', rectX) + .attr('y', rectY) + .attr('width', rectWidth) + .attr('height', rectHeight) + .attr('rx', 4) + if (transform) rectSelection.attr('transform', transform) + if (visibility) rectSelection.style('visibility', visibility) + rectSelection.lower() + + const arrowX = textBBox.x + textBBox.width + const arrowY = textBBox.y + (textBBox.height - arrowWidth) / 2 + + const arrowSelection = gSelection.selectAll('svg.attribute-label-arrow') + .data([1]) + .join((enter: any) => { + const arrow = enter.append('svg') + .attr('class', 'attribute-label-arrow') + .attr('viewBox', '0 0 24 24') + .attr('width', arrowWidth) + .attr('height', arrowWidth) + arrow.append('path').attr('d', 'm12 15-5-5h10z') + return arrow + }) + .attr('x', arrowX) + .attr('y', arrowY) + if (transform) arrowSelection.attr('transform', transform) + if (visibility) arrowSelection.style('visibility', visibility) +} diff --git a/v3/src/components/axis/components/axis-or-legend-attribute-menu.scss b/v3/src/components/axis/components/axis-or-legend-attribute-menu.scss index 20c3f0bf74..c335567d5b 100644 --- a/v3/src/components/axis/components/axis-or-legend-attribute-menu.scss +++ b/v3/src/components/axis/components/axis-or-legend-attribute-menu.scss @@ -4,39 +4,41 @@ touch-action: none; button:focus-visible { - outline: 2px solid vars.$focus-outline-color; - outline-offset: 2px; - border-radius: 2px; - } - - .axis-label-dropdown-arrow { - position: absolute; - right: -14px; - top: 50%; - transform: translateY(-50%); - width: 12px; - height: 12px; - color: vars.$icon-fill-dark; - pointer-events: none; - opacity: 0; - transition: opacity 0.15s ease; + outline: none; } +} - &:hover .axis-label-dropdown-arrow, - &:focus-within .axis-label-dropdown-arrow { - opacity: 1; +// Dropdown menu list styling (!important overrides Chakra UI's default MenuList styles) +.axis-legend-menu, .axis-legend-submenu { + border-radius: 4px !important; + box-shadow: 0 2px 6px 2px rgba(0, 0, 0, 0.25) !important; + padding: 6px 2px !important; + + [role="menuitem"] { + font-size: 14px; + font-weight: normal; + height: 30px; + padding: 0 10px; + + &:hover, &:focus { + background-color: vars.$charcoal-light-5; + } + + &:focus-visible { + @include vars.focus-outline; + background-color: transparent; + } + + // "Selected" (currently assigned) attribute + &[aria-checked="true"] { + background-color: vars.$charcoal-light-5; + } } -} -// For vertical axes, position the arrow below instead of to the right -.axis-legend-attribute-menu.left, -.axis-legend-attribute-menu.rightCat, -.axis-legend-attribute-menu.rightNumeric { - .attribute-label-menu .axis-label-dropdown-arrow { - right: 50%; - top: auto; - bottom: -14px; - transform: translateX(50%); + // Chakra uses this class for hover highlight + .chakra-menu__menuitem-option:hover, + .chakra-menu__menuitem:hover { + background-color: vars.$charcoal-light-5; } } diff --git a/v3/src/components/axis/components/axis-or-legend-attribute-menu.test.tsx b/v3/src/components/axis/components/axis-or-legend-attribute-menu.test.tsx index 2d4625d195..ad51eee0bb 100644 --- a/v3/src/components/axis/components/axis-or-legend-attribute-menu.test.tsx +++ b/v3/src/components/axis/components/axis-or-legend-attribute-menu.test.tsx @@ -184,6 +184,89 @@ describe("AxisOrLegendAttributeMenu", () => { }) }) + describe("CSS class relay to SVG target", () => { + let svgTarget: SVGGElement + + beforeEach(() => { + // Create an SVG element to serve as the target + const svg = document.createElementNS("http://www.w3.org/2000/svg", "svg") + svgTarget = document.createElementNS("http://www.w3.org/2000/svg", "g") + svg.appendChild(svgTarget) + document.body.appendChild(svg) + }) + + afterEach(() => { + svgTarget.closest("svg")?.remove() + }) + + it("adds 'hovered' class on pointer enter and removes on pointer leave", async () => { + const user = userEvent.setup() + renderMenu({ target: svgTarget }) + const overlay = screen.getByTestId("attribute-label-menu-bottom") + + await user.hover(overlay) + expect(svgTarget.classList.contains("hovered")).toBe(true) + + await user.unhover(overlay) + expect(svgTarget.classList.contains("hovered")).toBe(false) + }) + + it("adds 'focused' class on focus and removes on blur", () => { + renderMenu({ target: svgTarget }) + const button = screen.getByTestId("axis-legend-attribute-button-bottom") + + act(() => { button.focus() }) + expect(svgTarget.classList.contains("focused")).toBe(true) + + act(() => { button.blur() }) + expect(svgTarget.classList.contains("focused")).toBe(false) + }) + + it("adds 'menu-open' class when menu opens and removes when it closes", async () => { + const user = userEvent.setup() + renderMenu({ target: svgTarget }) + const button = screen.getByTestId("axis-legend-attribute-button-bottom") + + // Open menu + await user.click(button) + expect(svgTarget.classList.contains("menu-open")).toBe(true) + + // Close menu by clicking button again + await user.click(button) + expect(svgTarget.classList.contains("menu-open")).toBe(false) + }) + + it("removes 'focused' class when menu closes", async () => { + const user = userEvent.setup() + renderMenu({ target: svgTarget }) + const button = screen.getByTestId("axis-legend-attribute-button-bottom") + + // Open menu (which may set focused via focus events) + await user.click(button) + svgTarget.classList.add("focused") // simulate focus state + expect(svgTarget.classList.contains("menu-open")).toBe(true) + + // Close menu + await user.click(button) + expect(svgTarget.classList.contains("focused")).toBe(false) + }) + + it("cleans up all classes on unmount", async () => { + const user = userEvent.setup() + const { unmount } = renderMenu({ target: svgTarget }) + const overlay = screen.getByTestId("attribute-label-menu-bottom") + + // Set up some classes + await user.hover(overlay) + expect(svgTarget.classList.contains("hovered")).toBe(true) + + unmount() + expect(svgTarget.classList.contains("hovered")).toBe(false) + expect(svgTarget.classList.contains("focused")).toBe(false) + expect(svgTarget.classList.contains("menu-open")).toBe(false) + }) + }) + describe("menu rendering", () => { it("renders the menu list in the DOM", () => { renderMenu({ place: "bottom" }) diff --git a/v3/src/components/axis/components/axis-or-legend-attribute-menu.tsx b/v3/src/components/axis/components/axis-or-legend-attribute-menu.tsx index 2a7a63ec38..c4754c6406 100644 --- a/v3/src/components/axis/components/axis-or-legend-attribute-menu.tsx +++ b/v3/src/components/axis/components/axis-or-legend-attribute-menu.tsx @@ -1,7 +1,7 @@ import { clsx } from "clsx" import { observer } from "mobx-react-lite" import { Menu, MenuItem, MenuList, MenuButton, MenuDivider, Portal } from "@chakra-ui/react" -import React, { CSSProperties, useCallback, useRef, useState } from "react" +import React, { CSSProperties, useCallback, useEffect, useRef, useState } from "react" import { useDocumentContainerContext } from "../../../hooks/use-document-container-context" import { useFreeTileLayoutContext } from "../../../hooks/use-free-tile-layout-context" import { IUseDraggableAttribute, useDraggableAttribute } from "../../../hooks/use-drag-drop" @@ -22,7 +22,6 @@ import { GraphPlace } from "../../axis-graph-shared" import { graphPlaceToAttrRole } from "../../data-display/data-display-types" import { useDataConfigurationContext } from "../../data-display/hooks/use-data-configuration-context" -import DropdownArrow from "../../../assets/icons/arrow.svg" import RightArrow from "../../../assets/icons/arrow-right.svg" import "./axis-or-legend-attribute-menu.scss" @@ -152,7 +151,7 @@ export const AxisOrLegendAttributeMenu = observer(function AxisOrLegendAttribute }: IProps) { const containerRef = useDocumentContainerContext() const layout = useFreeTileLayoutContext() - const maxMenuHeight = `min(${layout?.height ?? 300}px, 50vh)` + const maxMenuHeight = `min(${layout?.height ?? 340}px, 50vh)` const dataConfiguration = useDataConfigurationContext() const isAttributeAllowed = dataConfiguration?.placeCanAcceptAttributeIDDrop ? (aPlace: GraphPlace, data: IDataSet, anAttrId: string) => @@ -182,7 +181,7 @@ export const AxisOrLegendAttributeMenu = observer(function AxisOrLegendAttribute const treatAs = (nativeType === 'date' && attrType === 'categorical') ? 'date' : ['numeric', 'date'].includes(attrType) ? "categorical" : "numeric" const overlayStyle: CSSProperties = { position: "absolute", ...useOverlayBounds({target, portal}) } - const buttonStyle: CSSProperties = { position: "absolute", width: "100%", height: "100%", color: "transparent" } + const buttonStyle: CSSProperties = { width: "100%", height: "100%", color: "transparent" } const menuRef = useRef(null) const mainMenuListRef = useRef(null) const [isMenuOpen, setIsMenuOpen] = useState(false) @@ -252,6 +251,21 @@ export const AxisOrLegendAttributeMenu = observer(function AxisOrLegendAttribute setIsMenuOpen(false) } + // Sync menu-open class with actual menu state, and clean up all visual state classes on close/unmount + useEffect(() => { + if (isMenuOpen) { + target?.classList.add("menu-open") + } else { + target?.classList.remove("menu-open") + target?.classList.remove("focused") + } + return () => { + target?.classList.remove("menu-open") + target?.classList.remove("focused") + target?.classList.remove("hovered") + } + }, [isMenuOpen, target]) + useOutsidePointerDown({ ref: menuRef, handler: handleCloseMenu, @@ -339,6 +353,22 @@ export const AxisOrLegendAttributeMenu = observer(function AxisOrLegendAttribute } } + const handlePointerEnter = () => { + target?.classList.add("hovered") + } + + const handlePointerLeave = () => { + target?.classList.remove("hovered") + } + + const handleFocusCapture = () => { + target?.classList.add("focused") + } + + const handleBlurCapture = () => { + target?.classList.remove("focused") + } + return (
@@ -348,14 +378,17 @@ export const AxisOrLegendAttributeMenu = observer(function AxisOrLegendAttribute } onCloseMenuRef.current = onClose return ( -
{attribute?.name} -