Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes widget header logic by introducing a shared stage_header helper and replacing inline header construction in each stage widget.
- Add
stage_headerfunction tomod.rsand import it into each widget. - Define a module-level
HEADER_TEXTconstant in each widget and remove duplicatedicon_buttonlogic. - Simplify containers and layouts across tone stack, preamp, power amp, filter, and compressor widgets.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/gui/widgets/tonestack.rs | Imported stage_header and replaced inline header code. |
| src/gui/widgets/preamp.rs | Ditto, plus added a TODO for background color. |
| src/gui/widgets/poweramp.rs | Ditto, simplified container layout. |
| src/gui/widgets/filter.rs | Ditto. |
| src/gui/widgets/compressor.rs | Ditto, cleaned up imports. |
| src/gui/widgets/mod.rs | Introduced the new stage_header helper function. |
Comments suppressed due to low confidence (4)
src/gui/widgets/mod.rs:52
- [nitpick] Consider adding a doc comment for
stage_headerto describe its purpose, parameters, and return value for clearer code maintenance.
pub fn stage_header(stage_name: &str, idx: usize, total_stages: usize) -> Element<Message> {
src/gui/widgets/tonestack.rs:7
- [nitpick] The constant name
HEADER_TEXTis quite generic. You might rename it toSTAGE_LABELorSTAGE_NAMEto better convey its purpose and avoid potential confusion.
const HEADER_TEXT: &str = "ToneStack";
src/gui/widgets/mod.rs:82
- The original headers placed the stage title before the buttons. The new order shows buttons first then text, which may unintentionally alter the UI layout. Confirm this change is intentional or revert to
textfollowed by the icon buttons.
row![move_up_btn, move_down_btn, remove_btn, text(header_text)]
src/gui/widgets/preamp.rs:50
- [nitpick] Add a tracking issue or remove this TODO to ensure the background color improvement isn’t forgotten or left without follow-up.
// TODO: Use a better background colour
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.