Fix/307 bug color scheme wahl funktioniert nicht#337
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a Dash callback to sync five color pickers with a selected color scheme, makes color pickers the single source of truth for node colors (visualization now reads picker values only), adjusts reset control outputs to exclude pickers, and updates CHANGELOG editorial content. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SchemeDropdown as Color Scheme Dropdown
participant ColorPickers as Color Pickers (Bus/Source/Sink/Storage/Converter)
participant Viz as update_visualization
rect rgb(240,248,255)
Note over SchemeDropdown,ColorPickers: Scheme selection updates pickers
User->>SchemeDropdown: select color scheme
SchemeDropdown-->>ColorPickers: update_color_pickers(color_scheme, reset_clicks)
ColorPickers-->>User: pickers show scheme colors
end
rect rgb(248,250,240)
Note over ColorPickers,Viz: Rendering reads pickers only
User->>Viz: trigger visualization update (edge_color, sizes, layout)
ColorPickers-->>Viz: bus_color, source_color, sink_color, storage_color, converter_color
Viz-->>User: render graph with picker-driven colors
end
sequenceDiagram
autonumber
actor User
participant ResetBtn as Reset Controls
participant SchemeDropdown as Color Scheme Dropdown
participant OtherControls as Edge/Size/Font/Layout
participant ColorPickers as Color Pickers
participant updateCP as update_color_pickers
rect rgb(255,248,240)
Note over ResetBtn,OtherControls: Reset emits non-picker defaults
User->>ResetBtn: click reset
ResetBtn-->>SchemeDropdown: set value to 'Default'
ResetBtn-->>OtherControls: set edge_color, node_size, font_size, layout
end
rect rgb(240,248,255)
Note over SchemeDropdown,ColorPickers: Scheme change triggers picker sync
SchemeDropdown-->>updateCP: color_scheme change / reset_clicks
updateCP-->>ColorPickers: set Bus/Source/Sink/Storage/Converter colors
ColorPickers-->>User: pickers reflect defaults
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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: 0
🧹 Nitpick comments (3)
flixopt/network_app.py (3)
538-560: Good addition: picker–scheme sync is clear and non-cyclic.Callback wiring looks correct and avoids feedback loops. Consider optionally syncing the edge color with the selected scheme as well (even if all schemes use the same gray), so “scheme” fully captures graph styling and “reset” isn’t the only way edges change.
Apply this localized change to include the edge picker in the same callback:
@app.callback( - [ + [ Output('bus-color-picker', 'value'), Output('source-color-picker', 'value'), Output('sink-color-picker', 'value'), Output('storage-color-picker', 'value'), Output('converter-color-picker', 'value'), + Output('edge-color-picker', 'value'), ], [Input('color-scheme-dropdown', 'value')], ) def update_color_pickers(color_scheme): @@ - return ( + return ( {'hex': colors['Bus']}, {'hex': colors['Source']}, {'hex': colors['Sink']}, {'hex': colors['Storage']}, {'hex': colors['Converter']}, + {'hex': '#808080'}, # keep edge consistent per scheme )
587-601: Harden color extraction against shape drift from DAQ ColorPicker.If dash_daq ever returns a string (e.g., “#RRGGBB”) instead of a dict,
.get('hex')will raise. Add a tiny helper to normalize values.Apply this diff:
def update_visualization( @@ - """Update visualization based on current color picker values""" + """Update visualization based on current color picker values""" + def _hex(val, default): + if not val: + return default + if isinstance(val, str): + return val + return val.get('hex', default) @@ - default_colors = VisualizationConfig.DEFAULT_COLORS - colors = { - 'Bus': bus_color.get('hex') if bus_color else default_colors['Bus'], - 'Source': source_color.get('hex') if source_color else default_colors['Source'], - 'Sink': sink_color.get('hex') if sink_color else default_colors['Sink'], - 'Storage': storage_color.get('hex') if storage_color else default_colors['Storage'], - 'Converter': converter_color.get('hex') if converter_color else default_colors['Converter'], - 'Other': default_colors['Other'], - } + default_colors = VisualizationConfig.DEFAULT_COLORS + colors = { + 'Bus': _hex(bus_color, default_colors['Bus']), + 'Source': _hex(source_color, default_colors['Source']), + 'Sink': _hex(sink_color, default_colors['Sink']), + 'Storage': _hex(storage_color, default_colors['Storage']), + 'Converter': _hex(converter_color, default_colors['Converter']), + 'Other': default_colors['Other'], + }
703-723: Reset UX: make reset atomic by also emitting picker values.Current reset relies on the scheme change to cascade picker updates; this can cause a brief visual mismatch on slow clients. Emitting picker values directly makes the reset one-shot and deterministic.
Apply this diff:
@app.callback( - [ + [ Output('color-scheme-dropdown', 'value'), Output('node-size-slider', 'value'), Output('font-size-slider', 'value'), Output('layout-dropdown', 'value'), Output('edge-color-picker', 'value'), + Output('bus-color-picker', 'value'), + Output('source-color-picker', 'value'), + Output('sink-color-picker', 'value'), + Output('storage-color-picker', 'value'), + Output('converter-color-picker', 'value'), ], [Input('reset-btn', 'n_clicks')], ) def reset_controls(n_clicks): @@ - if n_clicks and n_clicks > 0: - return ( - 'Default', # color scheme (will trigger color picker updates) - 90, # node size - 10, # font size - 'klay', # layout - {'hex': '#808080'}, # edge color - ) + if n_clicks and n_clicks > 0: + defaults = VisualizationConfig.DEFAULT_COLORS + return ( + 'Default', # color scheme + 90, # node size + 10, # font size + 'klay', # layout + {'hex': '#808080'}, # edge color + {'hex': defaults['Bus']}, + {'hex': defaults['Source']}, + {'hex': defaults['Sink']}, + {'hex': defaults['Storage']}, + {'hex': defaults['Converter']}, + )To verify no flicker remains, click “Reset to Defaults” repeatedly and observe that nodes/edges/colors change in a single render tick without intermediate states.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
CHANGELOG.md (4)
9-13: Fix typos and tighten wording in the guidance (HTML comment).Keeps the CI token “Whats Changed” intact while correcting pluralization and tone.
Apply this diff:
- Take Care: The CI will automatically append a "Whats Changed" section to the changelog. - This contains all Commits, PR's and Contributers. + Note: The CI will automatically append a "Whats Changed" section to the changelog. + This contains all commits, PRs, and contributors. @@ - Please remove all not relevant sections before releasing. + Please remove all irrelevant sections before releasing.Please confirm “Whats Changed” is the exact string your CI expects; if yes, keep it as-is.
30-30: Use standard Keep a Changelog header for Unreleased (drop date placeholder).Unreleased usually has no date.
Apply this diff:
-## [Unreleased] - ????-??-?? +## [Unreleased]
32-37: Trim empty sections before release; consider canonical section order.Remove empty headings at release time (as your note already says). Optional: order as Added, Changed, Deprecated, Removed, Fixed, Security for consistency with Keep a Changelog.
Also applies to: 41-44
38-39: Make the Fixed entry concise and reference the PR.Use imperative voice and add the PR number for traceability.
Apply this diff:
-- Fixed bug in the network_app that caused the color schemes to not work. Further, color pickers are now updated when selecting a color scheme. +- Fix color scheme selection in network_app; color pickers now update when a scheme is selected. (#337)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
CHANGELOG.md (6)
30-44: Tighten Unreleased section; avoid empty subsections and align with style.
- Drop empty subsections before release (Added/Changed/Deprecated/Known issues).
- Prefer plain "### Development" over italicized for consistency with other headings.
Apply:
-### *Development* +### Development
39-39: Make the Fixed entry precise and reference the PR for traceability.Name the module, note all five pickers, and link the PR/author to match prior entries.
Apply:
-- Fix color scheme selection in network_app; color pickers now update when a scheme is selected. +- Fix color scheme selection in `flixopt/network_app.py`: selecting a scheme now updates all five color pickers (single source of truth). [[#337](https://github.com/flixOpt/flixopt/pull/337) by [@FBumann](https://github.com/FBumann)]
45-49: Resolve wording inconsistency about “no changes or new features.”You have an “Added” entry below; rephrase to “no user-facing features.”
Apply:
-This update is a maintenance release to improve Code Quality, CI and update the dependencies. -There are no changes or new features. +Maintenance release focused on code quality, CI, and dependency updates. +No user-facing features were added in this version.
54-60: Normalize capitalization and wording in Development bullets.Use sentence case and consistent verbs.
Apply:
-- ruff format the whole Codebase -- Added renovate config -- Added pre-commit -- lint and format in CI -- improved CI -- Updated Dependencies -- Updated Issue Templates +- Format the whole codebase with ruff +- Add Renovate configuration +- Add pre-commit hooks +- Lint and format in CI +- Improve CI +- Update dependencies +- Update issue templates
26-26: Style consistency: avoid italics in headings (even inside comments).Match visible sections’ style for easier copy/paste later.
Apply:
-### *Development* +### Development
30-44: Optional: add guidance to Unreleased on release hygiene.Since CI appends “What’s Changed,” add a short reminder to remove any empty sections before tagging.
Would you like me to push a follow-up commit that removes empty Unreleased subsections as part of the release workflow?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
FBumann
deleted the
fix/307-bug-color-scheme-wahl-funktioniert-nicht
branch
1 participant
Description
Brief description of the changes in this PR.
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
New Features
Changes
Bug Fixes
Documentation