Make event icons responsible (#21141)

frontend/src/metabase/entities/timeline-events/forms.js

@@ -29,7 +29,6 @@ const createForm = ({ timelines }) => {
       name: "icon",
       title: t`Icon`,
       type: "select",
-      initial: "star",
       options: getTimelineIcons(),
       validate: validate.required(),
     },
@@ -40,7 +39,6 @@ const createForm = ({ timelines }) => {
     {
       name: "time_matters",
       type: "hidden",
-      initial: false,
     },
     {
       name: "timeline_id",

frontend/src/metabase/entities/timelines/forms.js

@@ -21,7 +21,6 @@ const createForm = () => {
       name: "icon",
       title: t`Default icon`,
       type: "select",
-      initial: "star",
       options: getTimelineIcons(),
       validate: validate.required(),
     },

frontend/src/metabase/timelines/collections/components/NewTimelineModal/NewTimelineModal.tsx

@@ -2,6 +2,7 @@ import React, { useCallback, useMemo } from "react";
 import { t } from "ttag";
 import Form from "metabase/containers/Form";
 import forms from "metabase/entities/timelines/forms";
+import { getDefaultTimelineIcon } from "metabase/lib/timelines";
 import { canonicalCollectionId } from "metabase/collections/utils";
 import ModalHeader from "metabase/timelines/common/components/ModalHeader";
 import { Collection, Timeline } from "metabase-types/api";
@@ -21,7 +22,10 @@ const NewTimelineModal = ({
   onClose,
 }: NewTimelineModalProps): JSX.Element => {
   const initialValues = useMemo(() => {
-    return { collection_id: canonicalCollectionId(collection.id) };
+    return {
+      collection_id: canonicalCollectionId(collection.id),
+      icon: getDefaultTimelineIcon(),
+    };
   }, [collection]);
 
   const handleSubmit = useCallback(

frontend/src/metabase/visualizations/components/LineAreaBarChart.css

@@ -249,36 +249,37 @@ text.value-label-white {
 
 /* timeline events */
 
-.dc-chart .events-axis .event-tick {
+.LineAreaBarChart .dc-chart .event-axis .event-tick {
   cursor: pointer;
   pointer-events: all;
 }
 
-.dc-chart .events-axis .event-tick .event-icon {
-  stroke: var(--color-text-light);
+.LineAreaBarChart .dc-chart .event-axis .event-icon {
   fill: var(--color-text-light);
+  stroke: var(--color-text-light);
+  shape-rendering: geometricPrecision;
 }
 
-.dc-chart .events-axis .event-tick.hover .event-icon {
-  stroke: var(--color-brand);
-  fill: var(--color-brand);
+.LineAreaBarChart .dc-chart .event-axis .event-text {
+  fill: var(--color-text-light);
 }
 
-.dc-chart .events-axis .event-tick text {
-  fill: var(--color-text-light);
+.LineAreaBarChart .dc-chart .event-axis .event-tick.hover .event-icon {
+  fill: var(--color-brand);
+  stroke: var(--color-brand);
 }
 
-.dc-chart .events-axis .event-tick.hover text {
+.LineAreaBarChart .dc-chart .event-axis .event-tick.hover .event-text {
   fill: var(--color-brand);
 }
 
-.dc-chart .event-line {
+.LineAreaBarChart .dc-chart .event-line {
   stroke: var(--color-brand);
   stroke-width: 2;
   opacity: 0.2;
   pointer-events: none;
 }
 
-.dc-chart .event-line.hover {
+.LineAreaBarChart .dc-chart .event-line.hover {
   opacity: 1;
 }

frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js

@@ -403,8 +403,8 @@ function onRenderAddTimelineEvents(
   },
 ) {
   renderEvents(chart, {
-    timelineEvents,
-    selectedTimelineEventIds,
+    events: timelineEvents,
+    selectedEventIds: selectedTimelineEventIds,
     xDomain,
     xInterval,
     isTimeseries,

frontend/src/metabase/visualizations/lib/timelines.js

@@ -4,16 +4,32 @@ import { ICON_PATHS } from "metabase/icon_paths";
 import { stretchTimeseriesDomain } from "./apply_axis";
 import timeseriesScale from "./timeseriesScale";
 
+const ICON_SIZE = 16;
 const ICON_SCALE = 0.45;
 const ICON_LARGE_SCALE = 0.35;
-const ICON_SIZE = 16;
 const ICON_X = -ICON_SIZE;
 const ICON_Y = 10;
+const ICON_DISTANCE = ICON_SIZE;
 const TEXT_X = 10;
-const TEXT_Y = ICON_Y + 8;
-const RECT_SIZE = 32;
+const TEXT_Y = 16;
+const TEXT_DISTANCE = ICON_SIZE * 1.75;
+const RECT_SIZE = ICON_SIZE * 2;
+
+function getXAxis(chart) {
+  return chart.svg().select(".axis.x");
+}
+
+function getBrush(chart) {
+  return chart.svg().select(".brush");
+}
 
-function getEventGroups(events, xInterval) {
+function getEventScale(chart, xDomain, xInterval) {
+  return timeseriesScale(xInterval)
+    .domain(stretchTimeseriesDomain(xDomain, xInterval))
+    .range([0, chart.effectiveWidth()]);
+}
+
+function getEventMapping(events, xInterval) {
   return _.groupBy(events, event =>
     event.timestamp
       .clone()
@@ -22,174 +38,225 @@ function getEventGroups(events, xInterval) {
   );
 }
 
-function getEventTicks(eventGroups) {
-  return Object.keys(eventGroups).map(value => new Date(parseInt(value)));
+function getEventDates(eventMapping) {
+  return Object.keys(eventMapping).map(value => new Date(parseInt(value)));
 }
 
-function getTranslateFromStyle(value) {
-  const style = value.replace("translate(", "").replace(")", "");
-  const [x, y] = style.split(",");
-  return [parseFloat(x), parseFloat(y)];
+function getEventGroups(eventMapping) {
+  return Object.values(eventMapping);
 }
 
-function getXAxis(chart) {
-  return chart.svg().select(".axis.x");
+function isSelected(events, selectedEventIds) {
+  return events.some(event => selectedEventIds.includes(event.id));
 }
 
-function getEventAxis(xAxis, xDomain, xInterval, eventTicks) {
-  const xAxisDomainLine = xAxis.select("path.domain").node();
-  const { width: axisWidth } = xAxisDomainLine.getBoundingClientRect();
-  xAxis.selectAll("event-axis").remove();
+function getIcon(events, eventIndex, eventScale, eventDates) {
+  if (isEventNarrow(eventIndex, eventScale, eventDates)) {
+    return "unknown";
+  } else {
+    return events.length === 1 ? events[0].icon : "star";
+  }
+}
 
-  const scale = timeseriesScale(xInterval)
-    .domain(stretchTimeseriesDomain(xDomain, xInterval))
-    .range([0, axisWidth]);
+function getIconPath(icon) {
+  return ICON_PATHS[icon].path ?? ICON_PATHS[icon];
+}
 
-  const eventsAxisGenerator = d3.svg
-    .axis()
-    .scale(scale)
-    .orient("bottom")
-    .ticks(eventTicks.length)
-    .tickValues(eventTicks);
+function getIconFillRule(icon) {
+  return ICON_PATHS[icon].attrs?.fillRule;
+}
 
-  const eventsAxis = xAxis
-    .append("g")
-    .attr("class", "events-axis")
-    .call(eventsAxisGenerator);
+function getIconTransform(icon) {
+  const scale = icon === "mail" ? ICON_LARGE_SCALE : ICON_SCALE;
+  return `scale(${scale}) translate(${ICON_X}, ${ICON_Y})`;
+}
+
+function getIconLabel(icon) {
+  return `${icon} icon`;
+}
 
-  eventsAxis.select("path.domain").remove();
-  return eventsAxis;
+function isEventWithin(eventIndex, eventScale, eventDates, eventDistance) {
+  const thisDate = eventDates[eventIndex];
+  const prevDate = eventDates[eventIndex - 1];
+  const nextDate = eventDates[eventIndex + 1];
+  const prevDistance = prevDate && eventScale(thisDate) - eventScale(prevDate);
+  const nextDistance = nextDate && eventScale(nextDate) - eventScale(thisDate);
+
+  return prevDistance < eventDistance || nextDistance < eventDistance;
+}
+
+function isEventNarrow(eventIndex, eventScale, eventDates) {
+  return isEventWithin(eventIndex, eventScale, eventDates, ICON_DISTANCE);
 }
 
-function renderEventTicks(
+function hasEventText(events, eventIndex, eventScale, eventDates) {
+  if (events.length > 1) {
+    return !isEventWithin(eventIndex, eventScale, eventDates, TEXT_DISTANCE);
+  } else {
+    return false;
+  }
+}
+
+function renderEventLines({
   chart,
-  {
-    eventAxis,
-    eventGroups,
-    selectedEventIds,
-    onHoverChange,
-    onOpenTimelines,
-    onSelectTimelineEvents,
-    onDeselectTimelineEvents,
-  },
-) {
-  const svg = chart.svg();
-  const brush = svg.select("g.brush");
-  const brushHeight = brush.select("rect.background").attr("height");
-
-  svg.selectAll(".event-tick").remove();
-  svg.selectAll(".event-line").remove();
-
-  Object.values(eventGroups).forEach(group => {
-    const defaultTick = eventAxis.select(".tick");
-    const transformStyle = defaultTick.attr("transform");
-    const [tickX] = getTranslateFromStyle(transformStyle);
-    defaultTick.remove();
-
-    const isSelected = group.some(event => selectedEventIds.includes(event.id));
-    const isOnlyOneEvent = group.length === 1;
-    const iconName = isOnlyOneEvent ? group[0].icon : "star";
-
-    const iconPath = ICON_PATHS[iconName].path
-      ? ICON_PATHS[iconName].path
-      : ICON_PATHS[iconName];
-    const iconScale = iconName === "mail" ? ICON_LARGE_SCALE : ICON_SCALE;
-
-    const eventLine = brush
-      .append("line")
-      .attr("class", "event-line")
-      .classed("hover", isSelected)
-      .attr("x1", tickX)
-      .attr("x2", tickX)
-      .attr("y1", "0")
-      .attr("y2", brushHeight);
-
-    const eventTick = eventAxis
-      .append("g")
-      .attr("class", "event-tick")
-      .classed("hover", isSelected)
-      .attr("transform", transformStyle);
-
-    const eventIcon = eventTick
-      .append("path")
-      .attr("class", "event-icon")
-      .attr("d", iconPath)
-      .attr("aria-label", `${iconName} icon`)
-      .attr("transform", `scale(${iconScale}) translate(${ICON_X},${ICON_Y})`);
-
-    eventTick
-      .append("rect")
-      .attr("fill", "none")
-      .attr("width", RECT_SIZE)
-      .attr("height", RECT_SIZE)
-      .attr("transform", `scale(${iconScale}) translate(${ICON_X}, ${ICON_Y})`);
-
-    if (!isOnlyOneEvent) {
-      eventTick
-        .append("text")
-        .text(group.length)
-        .attr("transform", `translate(${TEXT_X},${TEXT_Y})`);
-    }
-
-    eventTick
-      .on("mousemove", () => {
-        onHoverChange({
-          element: eventIcon.node(),
-          timelineEvents: group,
-        });
-        eventTick.classed("hover", true);
-        eventLine.classed("hover", true);
-      })
-      .on("mouseleave", () => {
-        onHoverChange(null);
-        eventTick.classed("hover", isSelected);
-        eventLine.classed("hover", isSelected);
-      })
-      .on("click", () => {
-        onOpenTimelines();
-        if (isSelected) {
-          onDeselectTimelineEvents(group);
-        } else {
-          onSelectTimelineEvents(group);
-        }
-      });
-  });
+  brush,
+  eventScale,
+  eventDates,
+  eventGroups,
+  selectedEventIds,
+}) {
+  const eventLines = brush.selectAll(".event-line").data(eventGroups);
+  const brushHeight = chart.effectiveHeight();
+  eventLines.exit().remove();
+
+  eventLines
+    .enter()
+    .append("line")
+    .attr("class", "event-line")
+    .classed("hover", d => isSelected(d, selectedEventIds))
+    .attr("x1", (d, i) => eventScale(eventDates[i]))
+    .attr("x2", (d, i) => eventScale(eventDates[i]))
+    .attr("y1", "0")
+    .attr("y2", brushHeight);
+}
+
+function renderEventTicks({
+  axis,
+  brush,
+  eventScale,
+  eventDates,
+  eventGroups,
+  selectedEventIds,
+  onHoverChange,
+  onOpenTimelines,
+  onSelectTimelineEvents,
+  onDeselectTimelineEvents,
+}) {
+  const eventAxis = axis.selectAll(".event-axis").data([eventGroups]);
+  const eventLines = brush.selectAll(".event-line").data(eventGroups);
+  eventAxis.exit().remove();
+
+  eventAxis
+    .enter()
+    .append("g")
+    .attr("class", "event-axis");
+
+  const eventTicks = eventAxis.selectAll(".event-tick").data(eventGroups);
+  eventTicks.exit().remove();
+
+  eventTicks
+    .enter()
+    .append("g")
+    .attr("class", "event-tick")
+    .classed("hover", d => isSelected(d, selectedEventIds))
+    .attr("transform", (d, i) => `translate(${eventScale(eventDates[i])}, 0)`);
+
+  eventTicks
+    .append("path")
+    .attr("class", "event-icon")
+    .attr("d", (d, i) => getIconPath(getIcon(d, i, eventScale, eventDates)))
+    .attr("fill-rule", (d, i) =>
+      getIconFillRule(getIcon(d, i, eventScale, eventDates)),
+    )
+    .attr("transform", (d, i) =>
+      getIconTransform(getIcon(d, i, eventScale, eventDates)),
+    )
+    .attr("aria-label", (d, i) =>
+      getIconLabel(getIcon(d, i, eventScale, eventDates)),
+    );
+
+  eventTicks
+    .append("rect")
+    .attr("fill", "none")
+    .attr("width", RECT_SIZE)
+    .attr("height", RECT_SIZE)
+    .attr("transform", (d, i) =>
+      getIconTransform(getIcon(d, i, eventScale, eventDates)),
+    );
+
+  eventTicks
+    .filter((d, i) => hasEventText(d, i, eventScale, eventDates))
+    .append("text")
+    .attr("class", "event-text")
+    .attr("transform", `translate(${TEXT_X},${TEXT_Y})`)
+    .text(d => d.length);
+
+  eventTicks
+    .on("mousemove", function(d) {
+      const eventTick = d3.select(this);
+      const eventIcon = eventTicks.filter(data => d === data);
+      const eventLine = eventLines.filter(data => d === data);
+
+      onHoverChange({ element: eventIcon.node(), timelineEvents: d });
+      eventTick.classed("hover", true);
+      eventLine.classed("hover", true);
+    })
+    .on("mouseleave", function(d) {
+      const eventTick = d3.select(this);
+      const eventLine = eventLines.filter(data => d === data);
+
+      onHoverChange(null);
+      eventTick.classed("hover", isSelected(d, selectedEventIds));
+      eventLine.classed("hover", isSelected(d, selectedEventIds));
+    })
+    .on("click", function(d) {
+      onOpenTimelines();
+
+      if (isSelected(d, selectedEventIds)) {
+        onDeselectTimelineEvents(d);
+      } else {
+        onSelectTimelineEvents(d);
+      }
+    });
 }
 
 export function renderEvents(
   chart,
   {
-    timelineEvents = [],
-    selectedTimelineEventIds = [],
-    xDomain,
-    xInterval,
-    isTimeseries,
+    events = [],
+    selectedEventIds = [],
+    xDomain = [],
+    xInterval = {},
     onHoverChange,
     onOpenTimelines,
     onSelectTimelineEvents,
     onDeselectTimelineEvents,
   },
 ) {
-  const xAxis = getXAxis(chart);
-  if (!xAxis || !isTimeseries || !timelineEvents.length) {
-    return;
-  }
+  const axis = getXAxis(chart);
+  const brush = getBrush(chart);
+  const eventScale = getEventScale(chart, xDomain, xInterval);
+  const eventMapping = getEventMapping(events, xInterval);
+  const eventDates = getEventDates(eventMapping);
+  const eventGroups = getEventGroups(eventMapping);
 
-  const eventGroups = getEventGroups(timelineEvents, xInterval);
-  const eventTicks = getEventTicks(eventGroups);
+  if (brush) {
+    renderEventLines({
+      chart,
+      brush,
+      eventScale,
+      eventDates,
+      eventGroups,
+      selectedEventIds,
+    });
+  }
 
-  const eventAxis = getEventAxis(xAxis, xDomain, xInterval, eventTicks);
-  renderEventTicks(chart, {
-    eventAxis,
-    eventGroups,
-    selectedEventIds: selectedTimelineEventIds,
-    onHoverChange,
-    onOpenTimelines,
-    onSelectTimelineEvents,
-    onDeselectTimelineEvents,
-  });
+  if (axis) {
+    renderEventTicks({
+      axis,
+      brush,
+      eventScale,
+      eventDates,
+      eventGroups,
+      selectedEventIds,
+      onHoverChange,
+      onOpenTimelines,
+      onSelectTimelineEvents,
+      onDeselectTimelineEvents,
+    });
+  }
 }
 
-export function hasEventAxis({ timelineEvents = [], xDomain, isTimeseries }) {
+export function hasEventAxis({ timelineEvents = [], isTimeseries }) {
   return isTimeseries && timelineEvents.length > 0;
 }