Skip to content

refactor(statmodel): consolidate submit and clear comparison matrix button to one namespace#132

Merged
tonywu1999 merged 2 commits intodevelfrom
refactor-submit
Nov 21, 2025
Merged

refactor(statmodel): consolidate submit and clear comparison matrix button to one namespace#132
tonywu1999 merged 2 commits intodevelfrom
refactor-submit

Conversation

@tonywu1999
Copy link
Copy Markdown
Contributor

@tonywu1999 tonywu1999 commented Nov 21, 2025

Summary by CodeRabbit

Release Notes

  • Improvements
    • Consolidated comparison panel controls with unified submit and clear buttons, replacing mode-specific controls with a single, consistent interface
    • Streamlined user workflow by reducing redundant buttons while maintaining full functionality for all comparison analysis modes

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 21, 2025

Walkthrough

This pull request consolidates multiple per-mode submit and clear button IDs into unified global IDs across the statmodel comparison panels. Constants, server logic, UI definitions, styling, and tests are updated to reference comparisons_submit and comparisons_clear instead of mode-specific buttons.

Changes

Cohort / File(s) Summary
Constants and declarations
R/constants.R
Removed mode-specific keys from NAMESPACE_STATMODEL (e.g., comparisons_custom_pairwise_submit, comparisons_all_vs_one_submit), added unified keys (comparisons_submit, comparisons_clear). Removed inline comments from comparison_mode_* constants in CONSTANTS_STATMODEL.
Server logic
R/module-statmodel-server.R
Replaced multiple per-mode trigger inputs with single inputNAMESPACE_STATMODEL$comparisons_submit for building the contrast matrix. Replaced multiple per-mode clear inputs with single inputNAMESPACE_STATMODEL$comparisons_clear for resetting. Preserved per-mode branching logic inside handlers.
UI button definitions
R/statmodel-ui-comparisons.R
Replaced mode-specific actionButton IDs in three comparison panels with unified comparisons_submit and comparisons_clear IDs across all panels. Button labels remain unchanged.
CSS styling
R/statmodel-ui-headers.R
Consolidated button style definitions, replacing references to eight mode-specific submit/clear IDs with single references to comparisons_submit and comparisons_clear.
Test updates
tests/testthat/test-module-statmodel-server.R, tests/testthat/test-statmodel-ui-options-contrasts.R, tests/testthat/test-utils-statmodel-server.R
Updated all test input bindings and assertions from mode-specific ID references to unified comparisons_submit and comparisons_clear keys. Added UI length assertion in one test block.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI as Comparison Panels
    participant Server
    participant Handler as Submit/Clear Handler
    participant Matrix as matrix_build()

    rect rgb(220, 240, 255)
    note over User,Matrix: Old Flow (Per-Mode Buttons)
    User->>UI: Click custom_pairwise_submit
    UI->>Server: Trigger custom_pairwise_submit input
    Server->>Handler: observeEvent(custom_pairwise_submit)
    Handler->>Matrix: Call matrix_build(mode="custom_pairwise")
    end

    rect rgb(240, 255, 240)
    note over User,Matrix: New Flow (Unified Submit Button)
    User->>UI: Click comparisons_submit
    UI->>Server: Trigger comparisons_submit input
    Server->>Handler: observeEvent(comparisons_submit)
    Handler->>Matrix: Call matrix_build() with per-mode branching
    Matrix->>Matrix: Route to mode-specific logic internally
    end

    rect rgb(255, 245, 220)
    note over User,Matrix: Reset Flow (Unified Clear Button)
    User->>UI: Click comparisons_clear
    UI->>Server: Trigger comparisons_clear input
    Server->>Handler: observeEvent(comparisons_clear)
    Handler->>Handler: Reset all UI elements
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

  • Server logic verification: Ensure the single comparisons_submit trigger correctly routes to per-mode branching logic and maintains validation flow for all three comparison modes.
  • Test coverage: Verify all test updates consistently reference the new unified IDs and that no mode-specific logic is inadvertently broken.
  • CSS selector correctness: Confirm that the consolidated style definitions still apply the intended styling to the unified button elements.
  • Event binding consistency: Cross-check that all UI panels use the unified IDs and that the server handlers capture all submission scenarios.

Possibly related PRs

  • MSstatsShiny#117: Defines the per-panel submit/clear UI components and helpers that are consolidated in this PR.
  • MSstatsShiny#130: Modifies the same NAMESPACE_STATMODEL and CONSTANTS_STATMODEL constants; this PR is a follow-up refactor that consolidates those per-panel keys.
  • MSstatsShiny#131: Refactors comparisons-related input names and server/UI handling in the same files, moving logic to use namespaced keys.

Suggested labels

Review effort 3/5, refactor, ui, consolidation

Poem

🐰 Buttons once scattered, now unified and neat,
One submit, one clear, the contrast mode is sweet!
Per-panel logic now routes through a single door,
Simpler flows, cleaner code—less confusion than before! 🌟

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: consolidating multiple per-mode submit/clear buttons into unified namespace constants across the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor-submit

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
R/statmodel-ui-comparisons.R (1)

45-46: LGTM! UI panels successfully consolidated to unified button IDs.

All four comparison panel builders now use the same comparisons_submit and comparisons_clear button IDs, enabling the single-trigger model in the server logic.

The button labels vary between "Add" and "Submit" for the submit action. While this may reflect different user interactions (adding to a list vs. submitting the full matrix), consider standardizing for consistency if the underlying behavior is identical.

Also applies to: 56-57, 65-66, 78-79

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d1bb53 and a12907d.

📒 Files selected for processing (7)
  • R/constants.R (1 hunks)
  • R/module-statmodel-server.R (3 hunks)
  • R/statmodel-ui-comparisons.R (3 hunks)
  • R/statmodel-ui-headers.R (1 hunks)
  • tests/testthat/test-module-statmodel-server.R (9 hunks)
  • tests/testthat/test-statmodel-ui-options-contrasts.R (4 hunks)
  • tests/testthat/test-utils-statmodel-server.R (1 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). (1)
  • GitHub Check: build
🔇 Additional comments (6)
tests/testthat/test-statmodel-ui-options-contrasts.R (1)

8-9: LGTM! Tests correctly updated for unified button IDs.

All four test cases now consistently verify the presence of the consolidated comparisons_submit and comparisons_clear button IDs instead of mode-specific variants. This aligns with the PR's objective to consolidate UI controls.

Also applies to: 22-23, 34-35, 49-50

tests/testthat/test-module-statmodel-server.R (1)

194-194: LGTM! Server tests updated consistently.

All test cases now trigger matrix building and validation through the unified NAMESPACE_STATMODEL$comparisons_submit input. The incremental counter pattern on lines 502 and 506 correctly tests duplicate submission handling.

Also applies to: 241-241, 284-284, 330-330, 379-379, 419-419, 457-457, 502-502, 506-506, 543-543

R/statmodel-ui-headers.R (1)

10-17: LGTM! CSS styling consolidated correctly.

The styling now targets the unified comparisons_submit and comparisons_clear buttons, eliminating redundant per-mode button styles. This aligns with the UI consolidation across all comparison panels.

tests/testthat/test-utils-statmodel-server.R (1)

348-348: LGTM! UI helper tests updated correctly.

The get_contrast_panel_ui tests now verify that all comparison modes return UI containing the unified comparisons_submit and comparisons_clear identifiers, confirming consistent button IDs across all panels.

Also applies to: 350-350, 356-356, 364-364

R/module-statmodel-server.R (1)

471-475: LGTM! Server logic correctly consolidated to unified triggers.

All reactive expressions (Rownames, check_cond, matrix_build) now respond to the single comparisons_submit trigger, while the clear observer uses comparisons_clear. Since the UI renders only one comparison panel at a time via conditional rendering (line 462-464), there are no conflicts from shared button IDs.

Also applies to: 488-492, 496-516, 519-523

R/constants.R (1)

2-12: LGTM! Constants correctly updated for consolidated button model.

The NAMESPACE_STATMODEL now includes the unified comparisons_submit and comparisons_clear keys while retaining mode-specific input identifiers (choice fields, names, weights) that remain distinct across comparison types. The CONSTANTS_STATMODEL comparison mode values are appropriately preserved.

@tonywu1999 tonywu1999 merged commit 6ad3d4a into devel Nov 21, 2025
3 checks passed
@tonywu1999 tonywu1999 deleted the refactor-submit branch November 21, 2025 15:05
tonywu1999 added a commit that referenced this pull request Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant