font size slider for visualizer #256
Conversation
ParticularlyPythonicBS
added
Maintenance
WalkthroughDynamic layout scaling for network graphs, a new Label Font Size slider in the visualization UI with per-node font-size wiring, exposure of master graph data to window in non-production, edge thickness switched to width-based values, CSS for controls, and a new unit test for commodity graph styling. Changes
Sequence Diagram(s)(Skipped — changes are dispersed across UI, layout logic, styling, and tests and do not introduce a single new multi-component sequential flow requiring diagramming.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@temoa/utilities/graph_utils.py`:
- Around line 234-241: The iterator all_edges is being consumed by num_edges =
sum(1 for _ in all_edges) causing the later loop for edge_tuple in all_edges to
see no items; instead materialize the iterable at the start (e.g., convert
all_edges to a list) before counting so num_edges and the subsequent loop both
operate on the same concrete sequence—update references around num_edges,
layout_radius, jitter_radius, and the for edge_tuple in all_edges loop to use
the materialized collection.
In `@temoa/utilities/network_vis_templates/graph_script.js`:
- Around line 35-42: The code attaches internal state to a global by assigning
window.__graph = { data, allNodesPrimary, allEdgesPrimary, allNodesSecondary,
allEdgesSecondary, optionsObject };; either remove this assignment before
production or gate it behind a clear flag/environment check and add a short
comment describing its debugging purpose. Update the block around the
window.__graph assignment (the symbol window.__graph and the variables data,
allNodesPrimary, allEdgesPrimary, allNodesSecondary, allEdgesSecondary,
optionsObject) to only export when a DEBUG/DEV_MODE flag is true or when
process.env.NODE_ENV !== 'production', and include a one-line comment stating
“Expose for debugging only — disable in production.”
In `@temoa/utilities/network_vis_templates/graph_template.html`:
- Around line 20-26: Summary: tighten the font-size slider range to a more
practical maximum. Update the input element with id "font-size-slider" to reduce
the max from 100 to 30 (and optionally add step="1" for finer control) while
keeping value="14" to match DEFAULT_VIS_OPTIONS; ensure any JS that reads the
slider (e.g., handlers referencing "font-size-slider") still accepts the new
range.
In `@tests/test_commodity_visualizer.py`:
- Around line 16-23: Replace the MagicMock usage in
tests/test_commodity_visualizer.py with a concrete NetworkModelData instance so
attribute lookups behave like the real class; instantiate NetworkModelData(),
then set network_data.physical_commodities = {'comm_inter'}, assign
network_data.source_commodities[(region, period)] = {'comm_source'} and
network_data.demand_commodities[(region, period)] = {'comm_demand'} and ensure
network_data.available_techs and network_data.tech_data use the class defaults
(or populate them minimally) before passing network_data into the
CommodityVisualizer test.
| <div class="legend-section"> | ||
| <h4>Visual Settings</h4> | ||
| <div class="control-group"> | ||
| <label for="font-size-slider">Label Font Size</label> | ||
| <input type="range" id="font-size-slider" min="6" max="100" value="14"> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Font size slider integration.
The slider is well-integrated with proper labeling and accessibility attributes. The default value of 14 correctly matches DEFAULT_VIS_OPTIONS.
One consideration: a max of 100 is quite large for label fonts. If most users will stay within 6-30, you could consider tightening the range for finer control, but the current range does allow for extreme zoom-out scenarios where larger labels might be useful.
🤖 Prompt for AI Agents
In `@temoa/utilities/network_vis_templates/graph_template.html` around lines 20 -
26, Summary: tighten the font-size slider range to a more practical maximum.
Update the input element with id "font-size-slider" to reduce the max from 100
to 30 (and optionally add step="1" for finer control) while keeping value="14"
to match DEFAULT_VIS_OPTIONS; ensure any JS that reads the slider (e.g.,
handlers referencing "font-size-slider") still accepts the new range.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@temoa/utilities/network_vis_templates/graph_script.js`:
- Around line 50-52: visualState.fontSize is hardcoded to 14 while optionsObject
(from options_json_str) may specify a different font size, causing a UI/render
mismatch; update the initialization so visualState.fontSize =
(optionsObject?.font_size ?? 14) and set the slider's initial value to that same
value during the UI setup (use the same slider variable or DOM element used
where the slider is created) so both the rendered network and the displayed
slider are synchronized; locate references to visualState, optionsObject, and
the slider initialization code (where the slider value is set) and replace the
hardcoded 14 with the derived value and apply it to the slider control.
| let visualState = { | ||
| fontSize: 14 | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and read the graph_script.js file
find . -name "graph_script.js" -type fRepository: TemoaProject/temoa
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Read the graph_script.js file to understand the full context
wc -l ./temoa/utilities/network_vis_templates/graph_script.jsRepository: TemoaProject/temoa
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Read the relevant sections of the file
cat -n ./temoa/utilities/network_vis_templates/graph_script.js | head -120 | tail -80Repository: TemoaProject/temoa
Length of output: 3994
🏁 Script executed:
#!/bin/bash
# Read the beginning of the file to understand how optionsObject is created
cat -n ./temoa/utilities/network_vis_templates/graph_script.js | head -50Repository: TemoaProject/temoa
Length of output: 2122
🏁 Script executed:
#!/bin/bash
# Search for font size references and how optionsObject is used
rg -n "fontSize|font.*size" ./temoa/utilities/network_vis_templates/graph_script.jsRepository: TemoaProject/temoa
Length of output: 709
🏁 Script executed:
#!/bin/bash
# Search for optionsObject usage and structure to see if it contains font settings
rg -n "optionsObject" ./temoa/utilities/network_vis_templates/graph_script.js | head -20Repository: TemoaProject/temoa
Length of output: 350
🏁 Script executed:
#!/bin/bash
# Look for where optionsRaw/options_json_str is generated, likely in a Python backend
find . -name "*.py" -type f | xargs rg -l "options_json_str|network_vis" | head -10Repository: TemoaProject/temoa
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Read the visualizer.py file to understand how options_json_str is generated
wc -l ./temoa/utilities/visualizer.pyRepository: TemoaProject/temoa
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Read the visualizer.py file to find options_json_str generation
rg -n "options_json_str|nodes.*font|edges.*font" ./temoa/utilities/visualizer.pyRepository: TemoaProject/temoa
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Read the visualizer.py file around line 242 and look for options construction
cat -n ./temoa/utilities/visualizer.py | sed -n '200,280p'Repository: TemoaProject/temoa
Length of output: 3792
🏁 Script executed:
#!/bin/bash
# Look at the DEFAULT_VIS_OPTIONS to understand the structure
cat -n ./temoa/utilities/visualizer.py | sed -n '272,315p'Repository: TemoaProject/temoa
Length of output: 1672
🏁 Script executed:
#!/bin/bash
# Check if the slider has a default value in the HTML template
rg -n "font-size-slider" ./temoa/utilities/network_vis_templates/ -A 2 -B 2Repository: TemoaProject/temoa
Length of output: 1411
🏁 Script executed:
#!/bin/bash
# Verify scope - check that optionsObject is defined before line 50
cat -n ./temoa/utilities/network_vis_templates/graph_script.js | sed -n '30,55p'Repository: TemoaProject/temoa
Length of output: 1021
Synchronize slider initial value with backend font size defaults to prevent UI/render mismatch.
visualState.fontSize and the slider are hardcoded to 14, but optionsObject (derived from backend options_json_str) may specify a different font size. When the network initializes, it uses the size from optionsObject, creating a mismatch where the actual render differs from the displayed slider value. Initialize both from optionsObject with a fallback to 14, and set the slider's initial value accordingly.
🛠️ Suggested fix
- let visualState = {
- fontSize: 14
- };
+ const parsedFontSize = parseInt(
+ optionsObject?.nodes?.font?.size ?? optionsObject?.edges?.font?.size ?? 14,
+ 10,
+ );
+ const defaultFontSize = Number.isFinite(parsedFontSize) ? parsedFontSize : 14;
+ let visualState = {
+ fontSize: defaultFontSize
+ };And at line 60:
const fontSizeSlider = document.getElementById('font-size-slider');
+ if (fontSizeSlider) fontSizeSlider.value = String(defaultFontSize);🤖 Prompt for AI Agents
In `@temoa/utilities/network_vis_templates/graph_script.js` around lines 50 - 52,
visualState.fontSize is hardcoded to 14 while optionsObject (from
options_json_str) may specify a different font size, causing a UI/render
mismatch; update the initialization so visualState.fontSize =
(optionsObject?.font_size ?? 14) and set the slider's initial value to that same
value during the UI setup (use the same slider variable or DOM element used
where the slider is created) so both the rendered network and the displayed
slider are synchronized; locate references to visualState, optionsObject, and
the slider initialization code (where the slider value is set) and replace the
hardcoded 14 with the derived value and apply it to the slider control.
ParticularlyPythonicBS
force-pushed
the
visualizer_improvements
branch
from
b5cb1b8 to
5f7428f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/source/default/static/Network_Graph_utopia_1990.html`:
- Around line 20-26: The range input with id "font-size-slider" is missing the
step attribute; update the element (identified by id "font-size-slider") to
include step="1" to match graph_template.html and ensure consistent behavior
across templates.
| <div class="legend-section"> | ||
| <h4>Visual Settings</h4> | ||
| <div class="control-group"> | ||
| <label for="font-size-slider">Label Font Size</label> | ||
| <input type="range" id="font-size-slider" min="6" max="100" value="14"> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Minor inconsistency: Missing step attribute on slider.
The font-size slider here is missing step="1" which is present in graph_template.html (line 24). While browsers default to step=1 for range inputs, adding it would maintain consistency across templates.
🔧 Proposed fix
- <input type="range" id="font-size-slider" min="6" max="100" value="14">
+ <input type="range" id="font-size-slider" min="6" max="100" step="1" value="14">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="legend-section"> | |
| <h4>Visual Settings</h4> | |
| <div class="control-group"> | |
| <label for="font-size-slider">Label Font Size</label> | |
| <input type="range" id="font-size-slider" min="6" max="100" value="14"> | |
| </div> | |
| </div> | |
| <div class="legend-section"> | |
| <h4>Visual Settings</h4> | |
| <div class="control-group"> | |
| <label for="font-size-slider">Label Font Size</label> | |
| <input type="range" id="font-size-slider" min="6" max="100" step="1" value="14"> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
In `@docs/source/default/static/Network_Graph_utopia_1990.html` around lines 20 -
26, The range input with id "font-size-slider" is missing the step attribute;
update the element (identified by id "font-size-slider") to include step="1" to
match graph_template.html and ensure consistent behavior across templates.
Added slider for label font size to the visualizer, added tests for special items in legends, updated visualizer embed in the docs
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.