From 0d01589f1017494cffb06f4f1c2d821a0e80d21d Mon Sep 17 00:00:00 2001 From: Miha Rekar Date: Fri, 30 Jan 2026 10:39:51 +0100 Subject: [PATCH 1/6] First pass --- Gemfile | 1 + Gemfile.lock | 2 + app/javascript/charts/index.js | 1 - app/javascript/charts/shot.js | 403 --------------- .../controllers/shot_charts_controller.js | 472 ++++++++++++++++++ app/models/shot_chart.rb | 4 - app/views/shots/compare.html.erb | 20 +- app/views/shots/show.html.erb | 37 +- 8 files changed, 489 insertions(+), 451 deletions(-) delete mode 100644 app/javascript/charts/shot.js create mode 100644 app/javascript/controllers/shot_charts_controller.js diff --git a/Gemfile b/Gemfile index 8f6d915df..2c2566e8b 100644 --- a/Gemfile +++ b/Gemfile @@ -57,6 +57,7 @@ group :development do gem "bundler-audit" gem "herb" gem "hotwire-spark" + gem "htmlbeautifier" gem "kamal" gem "letter_opener" gem "reactionview" diff --git a/Gemfile.lock b/Gemfile.lock index caa10b1fd..f6a7a9fab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -194,6 +194,7 @@ GEM listen rails (>= 7.0.0) zeitwerk + htmlbeautifier (1.4.3) i18n (1.14.8) concurrent-ruby (~> 1.0) image_processing (1.14.0) @@ -595,6 +596,7 @@ DEPENDENCIES factory_bot_rails herb hotwire-spark + htmlbeautifier image_processing importmap-rails inline_svg diff --git a/app/javascript/charts/index.js b/app/javascript/charts/index.js index b71bc1c32..2969116c3 100644 --- a/app/javascript/charts/index.js +++ b/app/javascript/charts/index.js @@ -1,2 +1 @@ -import "charts/shot" import "charts/stats" diff --git a/app/javascript/charts/shot.js b/app/javascript/charts/shot.js deleted file mode 100644 index db06365cc..000000000 --- a/app/javascript/charts/shot.js +++ /dev/null @@ -1,403 +0,0 @@ -import Highcharts from "highcharts" -import "highcharts-annotations" - -const deepMerge = (obj1, obj2) => { - const result = { ...obj1 } - for (const key in obj2) { - if (!obj2.hasOwnProperty(key)) return - - if (obj2[key] instanceof Object && obj1[key] instanceof Object) { - result[key] = deepMerge(obj1[key], obj2[key]) - } else { - result[key] = obj2[key] - } - } - return result -} - -const isObject = obj => obj && typeof obj === "object" - -const syncMouseEvents = element => { - element.addEventListener("mousemove", syncMouse) - element.addEventListener("touchstart", syncMouse) - element.addEventListener("touchmove", syncMouse) - element.addEventListener("mouseleave", mouseLeave) -} - -const getHoverPoint = (chart, e) => { - return chart.pointer.findNearestKDPoint(chart.series, true, chart.pointer.normalize(e)) -} - -function syncMouse(e) { - Highcharts.charts.forEach(chart => { - if (!isObject(chart) || this === chart.renderTo) return - - const hoverPoint = getHoverPoint(chart, e) - const hoverPoints = [] - - if (hoverPoint) { - chart.series.forEach(s => { - if (!s.visible) return - - const point = s.points.find(p => p.x === hoverPoint.x && !p.isNull) - if (isObject(point)) { - hoverPoints.push(point) - } - }) - } - - if (hoverPoints.length) { - chart.tooltip.refresh(hoverPoints) - chart.xAxis[0].drawCrosshair(e, hoverPoints[0]) - } - }) -} - -const mouseLeave = e => { - Highcharts.charts.forEach(chart => { - if (!isObject(chart)) return - - const hoverPoint = getHoverPoint(chart, e) - - if (hoverPoint) { - hoverPoint.onMouseOut() - chart.tooltip.hide(hoverPoint) - chart.xAxis[0].hideCrosshair() - } - }) -} - -const syncZoomReset = e => { - if (!e.resetSelection) return - - Highcharts.charts.forEach(chart => { - if (!isObject(chart) || !isObject(chart.resetZoomButton)) return - chart.resetZoomButton = chart.resetZoomButton.destroy() - }) -} - -const syncExtremes = function (e) { - if (e.trigger === "syncExtremes") return - - Highcharts.charts.forEach(chart => { - if (!isObject(chart) || chart === this.chart) return - if (!chart.xAxis[0].setExtremes) return - - chart.xAxis[0].setExtremes(e.min, e.max, undefined, false, { trigger: "syncExtremes" }) - if (isObject(chart.resetZoomButton) || e.min === undefined || e.max === undefined) return - - chart.showResetZoom() - }) -} - -const isDark = () => { - if (document.body.classList.contains("system")) { - return window.matchMedia && window.matchMedia("(prefers-color-scheme: dark)").matches - } else { - return document.body.classList.contains("dark") - } -} - -const getColors = () => { - if (isDark()) { - return { - background: "#171717", - label: "#999999", - gridLine: "#191919", - line: "#332914", - legend: "#cccccc", - legendHover: "#ffffff", - legendHidden: "#333333" - } - } else { - return { - background: "#ffffff", - label: "#666666", - gridLine: "#e6e6e6", - line: "#ccd6eb", - legend: "#333333", - legendHover: "#000000", - legendHidden: "#cccccc" - } - } -} - -const commonOptions = () => { - const colors = getColors() - - return { - accessibility: { enabled: false }, - animation: false, - title: false, - chart: { - zoomType: "x", - backgroundColor: colors.background, - events: { selection: syncZoomReset } - }, - xAxis: { - type: "datetime", - events: { setExtremes: syncExtremes }, - crosshair: true, - labels: { - style: { color: colors.label }, - formatter: function () { - if (this.value < 0) { - return `-${Highcharts.dateFormat("%M:%S", -this.value)}` - } else { - return Highcharts.dateFormat("%M:%S", this.value) - } - } - }, - gridLineColor: colors.gridLine, - lineColor: colors.line, - tickColor: colors.line - }, - yAxis: { - title: false, - labels: { style: { color: colors.label } }, - gridLineColor: colors.gridLine, - lineColor: colors.line, - tickColor: colors.line - }, - tooltip: { - animation: false, - shared: true, - borderRadius: 10, - shadow: false, - borderWidth: 0, - formatter: function (tooltip) { - let s - if (this.x < 0) { - s = [`-${Highcharts.dateFormat("%M:%S.%L", -this.x)}
`] - } else { - s = [`${Highcharts.dateFormat("%M:%S.%L", this.x)}
`] - } - - const visibleSeries = this.points.filter(point => point.series.visible) - if (visibleSeries.length === 2) { - const [series1, series2] = visibleSeries - const diff = Math.abs(series1.y - series2.y) - const formattedDiff = Highcharts.numberFormat(diff, 2) - s = s.concat(tooltip.bodyFormatter(visibleSeries)) - s.push(`Δ ${formattedDiff}`) - } else { - s = s.concat(tooltip.bodyFormatter(this.points)) - } - - return s - } - }, - legend: { - itemStyle: { color: colors.legend }, - itemHoverStyle: { color: colors.legendHover }, - itemHiddenStyle: { color: colors.legendHidden } - }, - plotOptions: { - series: { - animation: false, - marker: { - enabled: false, - states: { hover: { enabled: false } } - }, - states: { - hover: { enabled: false }, - inactive: { enabled: false } - } - } - }, - credits: { enabled: false } - } -} - -const extractStages = (field, timings) => { - for (const fieldData of window.shotData) { - if (fieldData.name === field) { - return timings.map(time => new Map(fieldData.data).get(time)) - } - } - return timings.map(() => 0) -} - -function setupInCupAnnotations(chart) { - const weightColor = chart.series.find(x => x.name === "Weight Flow").color - const timings = shotStages.map(x => x.value) - const weightFlow = extractStages("Weight Flow", timings) - const weight = extractStages("Weight", timings) - - let labels = [] - timings.forEach((timing, index) => { - const inCup = weight[index] - if (inCup > 0) { - labels.push({ - text: `${inCup}g`, - point: { x: timing, y: weightFlow[index], xAxis: 0, yAxis: 0 }, - y: -30, - x: -30, - allowOverlap: true, - style: { color: weightColor }, - borderColor: weightColor - }) - } - }) - - chart.inCupAnnotation = chart.addAnnotation({ - draggable: false, - labels: labels, - labelOptions: { shape: "connector" } - }) - - chart.renderer.text(' - - - <% end %> -
+ <% end %>
-
+
-
+
<% end %> diff --git a/app/views/shots/show.html.erb b/app/views/shots/show.html.erb index 39717d843..4806767e6 100644 --- a/app/views/shots/show.html.erb +++ b/app/views/shots/show.html.erb @@ -42,12 +42,15 @@ <% if @chart %> -
+
-
+
-
+
<% end %> From 866a5b86d92c52d195c8f25cc265ecaa33abe6e3 Mon Sep 17 00:00:00 2001 From: Miha Rekar Date: Fri, 30 Jan 2026 11:01:05 +0100 Subject: [PATCH 3/6] Move stats too --- app/javascript/application.js | 1 - app/javascript/charts/index.js | 1 - app/javascript/charts/stats.js | 30 ---------- .../controllers/stats_charts_controller.js | 56 +++++++++++++++++++ .../autocomplete_coffee_bags.html.erb | 2 +- app/views/shots/compare.html.erb | 6 +- app/views/shots/show.html.erb | 6 +- app/views/stats/index.html.erb | 13 ++--- config/importmap.rb | 1 - 9 files changed, 69 insertions(+), 47 deletions(-) delete mode 100644 app/javascript/charts/index.js delete mode 100644 app/javascript/charts/stats.js create mode 100644 app/javascript/controllers/stats_charts_controller.js diff --git a/app/javascript/application.js b/app/javascript/application.js index a94a51626..f10eb1ff2 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -3,5 +3,4 @@ import "@hotwired/turbo-rails" import "@rails/activestorage" import "controllers" -import "charts" import "custom" diff --git a/app/javascript/charts/index.js b/app/javascript/charts/index.js deleted file mode 100644 index 2969116c3..000000000 --- a/app/javascript/charts/index.js +++ /dev/null @@ -1 +0,0 @@ -import "charts/stats" diff --git a/app/javascript/charts/stats.js b/app/javascript/charts/stats.js deleted file mode 100644 index f6de74093..000000000 --- a/app/javascript/charts/stats.js +++ /dev/null @@ -1,30 +0,0 @@ -import Highcharts from "highcharts" - -document.addEventListener("turbo:load", function () { - const options = { - accessibility: { enabled: false }, - chart: { - zoomType: "x", - height: 500 - }, - xAxis: { - type: "datetime" - }, - title: false, - yAxis: { - title: false - }, - credits: { - enabled: false - } - } - if (document.getElementById("shot-uploaded-chart")) { - const chartOptions = { ...options, series: window.uploadedChartData } - Highcharts.chart("shot-uploaded-chart", chartOptions) - } - - if (document.getElementById("shot-user-chart")) { - const chartOptions = { ...options, series: window.userChartData } - Highcharts.chart("shot-user-chart", chartOptions) - } -}) diff --git a/app/javascript/controllers/stats_charts_controller.js b/app/javascript/controllers/stats_charts_controller.js new file mode 100644 index 000000000..7b6b1be57 --- /dev/null +++ b/app/javascript/controllers/stats_charts_controller.js @@ -0,0 +1,56 @@ +import { Controller } from "@hotwired/stimulus" +import Highcharts from "highcharts" + +export default class extends Controller { + static targets = ["uploadedChart", "userChart"] + static values = { + uploadedChartData: Array, + userChartData: Array + } + + connect() { + this.charts = [] + this.initializeCharts() + } + + disconnect() { + this.charts.forEach(chart => chart.destroy()) + this.charts = [] + } + + initializeCharts() { + const options = { + accessibility: { enabled: false }, + chart: { + zoomType: "x", + height: 500 + }, + xAxis: { + type: "datetime" + }, + title: false, + yAxis: { + title: false + }, + credits: { + enabled: false + } + } + + if (this.hasUploadedChartTarget && this.uploadedChartDataValue.length > 0) { + const chart = Highcharts.chart(this.uploadedChartTarget, { + ...options, + series: this.uploadedChartDataValue + }) + this.charts.push(chart) + } + + if (this.hasUserChartTarget && this.userChartDataValue.length > 0) { + const chart = Highcharts.chart(this.userChartTarget, { + ...options, + series: this.userChartDataValue + }) + this.charts.push(chart) + } + } +} diff --git a/app/views/canonical/autocomplete_coffee_bags.html.erb b/app/views/canonical/autocomplete_coffee_bags.html.erb index b8ebb5359..48914da1d 100644 --- a/app/views/canonical/autocomplete_coffee_bags.html.erb +++ b/app/views/canonical/autocomplete_coffee_bags.html.erb @@ -3,7 +3,7 @@
  • <%= coffee_bag.name %><% if params[:roaster_id].blank? %> (<%= coffee_bag.canonical_roaster.name %>) <% else %> -
    +
    <% end %>
    • <% end %> diff --git a/app/views/shots/compare.html.erb b/app/views/shots/compare.html.erb index eb7ccd413..5794ea29d 100644 --- a/app/views/shots/compare.html.erb +++ b/app/views/shots/compare.html.erb @@ -10,10 +10,10 @@
    <% if @chart %>
    + data-shot-charts-temperature-data-value="<%= (@chart.temperature_chart || []).to_json %>" + data-shot-charts-comparison-data-value="<%= (@chart.comparison_data || {}).to_json %>"> <% if Current.user&.premium? %>
    diff --git a/app/views/shots/show.html.erb b/app/views/shots/show.html.erb index 4806767e6..a994206ed 100644 --- a/app/views/shots/show.html.erb +++ b/app/views/shots/show.html.erb @@ -43,9 +43,9 @@
    <% if @chart %>
    + data-shot-charts-shot-data-value="<%= (@chart.shot_chart || []).to_json %>" + data-shot-charts-shot-stages-value="<%= (@chart.stages || []).to_json %>" + data-shot-charts-temperature-data-value="<%= (@chart.temperature_chart || []).to_json %>">
    diff --git a/app/views/stats/index.html.erb b/app/views/stats/index.html.erb index e22ced1ff..99c08aa3b 100644 --- a/app/views/stats/index.html.erb +++ b/app/views/stats/index.html.erb @@ -5,15 +5,14 @@
    -
    +
    Visualizer has <%= @user_count %> users who've uploaded <%= @shot_count %> shots -
    -
    +
    +
    - diff --git a/config/importmap.rb b/config/importmap.rb index 5da1533a8..f9154fc5d 100644 --- a/config/importmap.rb +++ b/config/importmap.rb @@ -24,5 +24,4 @@ pin_all_from "app/javascript/channels", under: "channels" pin_all_from "app/javascript/controllers", under: "controllers" pin_all_from "app/javascript/helpers", under: "helpers" -pin_all_from "app/javascript/charts", under: "charts" pin_all_from "app/javascript/custom", under: "custom" From 518125f21732ac6e52bd187f38ab2a95e2f92b4c Mon Sep 17 00:00:00 2001 From: Miha Rekar Date: Fri, 30 Jan 2026 11:36:37 +0100 Subject: [PATCH 4/6] Address review --- app/javascript/controllers/shot_charts_controller.js | 4 +++- app/javascript/controllers/stats_charts_controller.js | 4 +++- app/javascript/helpers/object_helpers.js | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/javascript/controllers/shot_charts_controller.js b/app/javascript/controllers/shot_charts_controller.js index bc8a52307..71aa1f0e9 100644 --- a/app/javascript/controllers/shot_charts_controller.js +++ b/app/javascript/controllers/shot_charts_controller.js @@ -48,7 +48,9 @@ export default class extends Controller { } destroyCharts() { - this.charts.forEach(chart => chart.destroy()) + this.charts.forEach(chart => { + chart.destroy() + }) this.charts = [] } diff --git a/app/javascript/controllers/stats_charts_controller.js b/app/javascript/controllers/stats_charts_controller.js index 7b6b1be57..0ac25a4ed 100644 --- a/app/javascript/controllers/stats_charts_controller.js +++ b/app/javascript/controllers/stats_charts_controller.js @@ -14,7 +14,9 @@ export default class extends Controller { } disconnect() { - this.charts.forEach(chart => chart.destroy()) + this.charts.forEach(chart => { + chart.destroy() + }) this.charts = [] } diff --git a/app/javascript/helpers/object_helpers.js b/app/javascript/helpers/object_helpers.js index 1394aedcd..4e0f7dd11 100644 --- a/app/javascript/helpers/object_helpers.js +++ b/app/javascript/helpers/object_helpers.js @@ -1,7 +1,7 @@ export const deepMerge = (obj1, obj2) => { const result = { ...obj1 } for (const key in obj2) { - if (!obj2.hasOwnProperty(key)) return + if (!Object.hasOwn(obj2, key)) continue if (obj2[key] instanceof Object && obj1[key] instanceof Object) { result[key] = deepMerge(obj1[key], obj2[key]) From 5af724c32f3eb3014718f3d2fe822ce3258f6017 Mon Sep 17 00:00:00 2001 From: Miha Rekar Date: Fri, 30 Jan 2026 11:50:52 +0100 Subject: [PATCH 5/6] Fix shot chart annotation toggle Use a stable plotline id and render the toggle button with valid HTML so it displays and updates correctly. --- .../controllers/shot_charts_controller.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/app/javascript/controllers/shot_charts_controller.js b/app/javascript/controllers/shot_charts_controller.js index 71aa1f0e9..31e225036 100644 --- a/app/javascript/controllers/shot_charts_controller.js +++ b/app/javascript/controllers/shot_charts_controller.js @@ -28,7 +28,7 @@ export default class extends Controller { initializeCharts() { this.destroyCharts() - this.shotStagesNormalized = this.shotStagesValue.map(x => ({ ...x, id: x })) + this.shotStagesNormalized = this.shotStagesValue.map(x => ({ ...x, id: String(x.value) })) if (this.hasShotChartTarget && this.shotDataValue.length) { const chart = this.drawShotChart(this.shotChartTarget) @@ -149,19 +149,28 @@ export default class extends Controller { labelOptions: { shape: "connector" } }) - chart.renderer.text('', + 50, + 35, + true + ) + .attr({ zIndex: 3 }) + .add() chart.annotationVisible = true - document.getElementById("remove-annotations").addEventListener("click", function () { + const toggleButton = chart.annotationButton.element.querySelector("button") + toggleButton?.addEventListener("click", function () { if (chart.annotationVisible) { chart.controller.destroyShotStages() chart.annotations[0].graphic.hide() chart.annotationVisible = false - this.innerHTML = "Show annotations" + toggleButton.textContent = "Show annotations" } else { chart.controller.drawShotStages() chart.annotations[0].graphic.show() chart.annotationVisible = true - this.innerHTML = "Hide annotations" + toggleButton.textContent = "Hide annotations" } }) } From 62400b6c137893cbd136a8ae55f9125b42d43e39 Mon Sep 17 00:00:00 2001 From: Miha Rekar Date: Fri, 30 Jan 2026 11:59:10 +0100 Subject: [PATCH 6/6] Harden shot chart annotations Clean up annotation toggle handlers when annotations are removed and avoid implicit returns in plotline rendering. --- .../controllers/shot_charts_controller.js | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/app/javascript/controllers/shot_charts_controller.js b/app/javascript/controllers/shot_charts_controller.js index 31e225036..7f143ed35 100644 --- a/app/javascript/controllers/shot_charts_controller.js +++ b/app/javascript/controllers/shot_charts_controller.js @@ -160,7 +160,11 @@ export default class extends Controller { .add() chart.annotationVisible = true const toggleButton = chart.annotationButton.element.querySelector("button") - toggleButton?.addEventListener("click", function () { + if (!toggleButton) return + + chart.annotationToggleHandler = () => { + if (!chart.annotations || !chart.annotations[0] || !chart.annotations[0].graphic) return + if (chart.annotationVisible) { chart.controller.destroyShotStages() chart.annotations[0].graphic.hide() @@ -172,7 +176,9 @@ export default class extends Controller { chart.annotationVisible = true toggleButton.textContent = "Hide annotations" } - }) + } + + toggleButton.addEventListener("click", chart.annotationToggleHandler) } updateInCupVisibility(chart) { @@ -183,6 +189,16 @@ export default class extends Controller { if (annotation && !isVisible) { chart.removeAnnotation(annotation) chart.inCupAnnotation = null + chart.annotationVisible = false + if (chart.annotationButton) { + const toggleButton = chart.annotationButton.element.querySelector("button") + if (toggleButton && chart.annotationToggleHandler) { + toggleButton.removeEventListener("click", chart.annotationToggleHandler) + } + chart.annotationButton.destroy() + chart.annotationButton = null + chart.annotationToggleHandler = null + } } else if (this.shotStagesNormalized.length > 0 && !annotation && isVisible) { this.setupInCupAnnotations(chart) } @@ -221,7 +237,9 @@ export default class extends Controller { drawShotStages() { this.charts.forEach(chart => { if (isObject(chart)) { - this.shotStagesNormalized.forEach(x => chart.xAxis[0].addPlotLine(x)) + this.shotStagesNormalized.forEach(x => { + chart.xAxis[0].addPlotLine(x) + }) } }) }