diff --git a/e2e/support/helpers/e2e-visual-tests-helpers.js b/e2e/support/helpers/e2e-visual-tests-helpers.js index 45462f8a95df5e3da61918101fa02a859d0b6d40..f3a8e01db5589fb14776d7a419140781ff62a9c9 100644 --- a/e2e/support/helpers/e2e-visual-tests-helpers.js +++ b/e2e/support/helpers/e2e-visual-tests-helpers.js @@ -42,10 +42,10 @@ export function getXYTransform(element) { return { x, y }; } -export function echartsIcon(name, color = undefined) { +export function echartsIcon(name, isSelected = false) { const iconSvg = setSvgColor( Icons[name].source, - color ?? getColor("text-light"), + getColor(isSelected ? "brand" : "text-light"), ); const dataUri = svgToDataUri(iconSvg); diff --git a/e2e/test/scenarios/organization/timelines-question.cy.spec.js b/e2e/test/scenarios/organization/timelines-question.cy.spec.js index 9943c25a93eb5c123337437c2b08b49fd8e23297..06c2fd8f24bd84624d32a793ab86c5ace8bf3ae3 100644 --- a/e2e/test/scenarios/organization/timelines-question.cy.spec.js +++ b/e2e/test/scenarios/organization/timelines-question.cy.spec.js @@ -392,6 +392,52 @@ describe("scenarios > organization > timelines > question", () => { echartsIcon("warning").should("not.exist"); }); + + it("should color the event icon when hovering", () => { + cy.createTimelineWithEvents({ + timeline: { name: "Releases" }, + events: [ + { name: "RC1", timestamp: "2024-10-20T00:00:00Z", icon: "star" }, + ], + }); + + visitQuestion(ORDERS_BY_YEAR_QUESTION_ID); + + echartsIcon("star").should("be.visible"); + echartsIcon("star").realHover(); + echartsIcon("star", true).should("be.visible"); + }); + + it("should open the sidebar when clicking an event icon", () => { + cy.createTimelineWithEvents({ + timeline: { name: "Releases" }, + events: [ + { name: "RC1", timestamp: "2024-10-20T00:00:00Z", icon: "star" }, + ], + }); + + visitQuestion(ORDERS_BY_YEAR_QUESTION_ID); + + echartsIcon("star").should("be.visible"); + echartsIcon("star").realClick(); + + // event should be selected in sidebar + timelineEventCard("RC1").should("be.visible"); + timelineEventCard("RC1").should( + "have.css", + "border-left", + "4px solid rgb(80, 158, 227)", + ); + + // after clicking the icon again, it should be deselected in sidebar + echartsIcon("star", true).click(); + timelineEventCard("RC1").should("be.visible"); + timelineEventCard("RC1").should( + "have.css", + "border-left", + "4px solid rgba(0, 0, 0, 0)", + ); + }); }); describe("as readonly user", () => { @@ -430,10 +476,12 @@ describe("scenarios > organization > timelines > question", () => { }); }); +function timelineEventCard(eventName) { + return cy.findByText(eventName).closest("[aria-label=Timeline event card]"); +} + function toggleEventVisibility(eventName) { - cy.findByText(eventName) - .closest("[aria-label=Timeline event card]") - .within(() => { - cy.findByRole("checkbox").click(); - }); + timelineEventCard(eventName).within(() => { + cy.findByRole("checkbox").click(); + }); } diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/constants/dataset.ts b/frontend/src/metabase/visualizations/echarts/cartesian/constants/dataset.ts index 693cf73e63d45bcaf26eefb5ce95af31e9dcf6ac..d638b37ec1710db5d28572f58410dd0c1db5b73c 100644 --- a/frontend/src/metabase/visualizations/echarts/cartesian/constants/dataset.ts +++ b/frontend/src/metabase/visualizations/echarts/cartesian/constants/dataset.ts @@ -29,3 +29,7 @@ export const TICKS_INTERVAL_THRESHOLD = 3; export const ECHARTS_CATEGORY_AXIS_NULL_VALUE = `${NULL_CHAR}_NULL` as const; export const GOAL_LINE_SERIES_ID = `${NULL_CHAR}_goal_line` as const; + +export const TIMELINE_EVENT_SERIES_ID = `${NULL_CHAR}_timeline_events`; + +export const TIMELINE_EVENT_DATA_NAME = `${NULL_CHAR}_timeline_event`; diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/timeline-events/option.ts b/frontend/src/metabase/visualizations/echarts/cartesian/timeline-events/option.ts index 26e887996f6d77e5e94bcc1cae9b72ccf08ac999..7249d3d46d22487ae70405b3565326e5ab3321de 100644 --- a/frontend/src/metabase/visualizations/echarts/cartesian/timeline-events/option.ts +++ b/frontend/src/metabase/visualizations/echarts/cartesian/timeline-events/option.ts @@ -8,6 +8,11 @@ import type { TimelineEventsModel } from "metabase/visualizations/echarts/cartes import type { RenderingContext } from "metabase/visualizations/types"; import type { TimelineEventId } from "metabase-types/api"; +import { + TIMELINE_EVENT_DATA_NAME, + TIMELINE_EVENT_SERIES_ID, +} from "../constants/dataset"; + // TODO: change to GraalVM supported implementation export const setSvgColor = (svgString: string, color: string) => { // Parse the SVG string into a DOM element @@ -55,7 +60,7 @@ export const getTimelineEventsSeries = ( const dataUri = svgToImageUri(iconSvg); return { - name: "timeline-event", + name: TIMELINE_EVENT_DATA_NAME, xAxis: date, symbolSize: 16, symbolOffset: [0, 12], @@ -77,7 +82,7 @@ export const getTimelineEventsSeries = ( }); return { - id: "timeline-events", + id: TIMELINE_EVENT_SERIES_ID, animation: false, type: "line", data: [], @@ -93,6 +98,17 @@ export const getTimelineEventsSeries = ( opacity: 1, }, }, + emphasis: { + lineStyle: { + color: getColor("brand"), + }, + label: { + color: getColor("brand"), + }, + itemStyle: { + color: getColor("brand"), + }, + }, symbol: "none", lineStyle: { type: "solid", diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/events.ts b/frontend/src/metabase/visualizations/visualizations/CartesianChart/events.ts index 98f74fe5ae5407f5185595e6c8090352f084c29f..a64288144454bfecca38badfc87b4f7d96d4b9c8 100644 --- a/frontend/src/metabase/visualizations/visualizations/CartesianChart/events.ts +++ b/frontend/src/metabase/visualizations/visualizations/CartesianChart/events.ts @@ -459,12 +459,8 @@ export const getTimelineEventsHoverData = ( ); const element = event.event.event.target as Element; - if (element?.nodeName !== "image") { - return null; - } - return { - element, + element: element?.nodeName === "image" ? element : undefined, timelineEvents: hoveredTimelineEvents, }; }; diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-chart-events.ts b/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-chart-events.ts index d4a8052203ab8595ddb0e8ec316f16129e280f75..a518d2b5b3da3c9b3adf408db5de8b56b02fd38d 100644 --- a/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-chart-events.ts +++ b/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-chart-events.ts @@ -5,6 +5,7 @@ import { useCallback, useEffect, useMemo, useRef } from "react"; import { GOAL_LINE_SERIES_ID, ORIGINAL_INDEX_DATA_KEY, + TIMELINE_EVENT_DATA_NAME, } from "metabase/visualizations/echarts/cartesian/constants/dataset"; import type { BaseCartesianChartModel, @@ -84,7 +85,7 @@ export const useChartEvents = ( return; } - if (timelineEventsModel && event.name === "timeline-event") { + if (timelineEventsModel && event.name === TIMELINE_EVENT_DATA_NAME) { const eventData = getTimelineEventsHoverData( timelineEventsModel, event, @@ -124,7 +125,7 @@ export const useChartEvents = ( handler: (event: EChartsSeriesMouseEvent) => { const clickData = getSeriesClickData(chartModel, settings, event); - if (timelineEventsModel && event.name === "timeline-event") { + if (timelineEventsModel && event.name === TIMELINE_EVENT_DATA_NAME) { onOpenTimelines?.(); const clickedTimelineEvents = getTimelineEventsForEvent( diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-models-and-option.ts b/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-models-and-option.ts index a929d406e197327e1d2e1aeb2a0e77291187eaf3..c8731464f456724575f29a378f3890ca4c2372df 100644 --- a/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-models-and-option.ts +++ b/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-models-and-option.ts @@ -112,6 +112,19 @@ export function useModelsAndOption({ [chartModel.seriesModels, hovered], ); + const selectedOrHoveredTimelineEventIds = useMemo(() => { + const ids = []; + + if (selectedTimelineEventIds != null) { + ids.push(...selectedTimelineEventIds); + } + if (hovered?.timelineEvents != null) { + ids.push(...hovered.timelineEvents.map(e => e.id)); + } + + return ids; + }, [selectedTimelineEventIds, hovered?.timelineEvents]); + const option = useMemo(() => { if (width === 0 || height === 0) { return {}; @@ -124,7 +137,7 @@ export function useModelsAndOption({ width, chartMeasurements, timelineEventsModel, - selectedTimelineEventIds ?? [], + selectedOrHoveredTimelineEventIds, settings, isPlaceholder ?? false, renderingContext, @@ -134,7 +147,7 @@ export function useModelsAndOption({ chartModel as CartesianChartModel, chartMeasurements, timelineEventsModel, - selectedTimelineEventIds ?? [], + selectedOrHoveredTimelineEventIds, settings, width, isPlaceholder ?? false, @@ -147,13 +160,13 @@ export function useModelsAndOption({ chartModel, chartMeasurements, renderingContext, - selectedTimelineEventIds, settings, timelineEventsModel, hoveredSeriesDataKey, width, height, isPlaceholder, + selectedOrHoveredTimelineEventIds, ]); return { chartModel, timelineEventsModel, option };