Skip to content

Added more help to the pmultiqc service #581

Merged
ypriverol merged 2 commits intomainfrom
dev
Jan 21, 2026
Merged

Added more help to the pmultiqc service #581
ypriverol merged 2 commits intomainfrom
dev

Conversation

@ypriverol
Copy link
Member

@ypriverol ypriverol commented Jan 21, 2026

Pull Request

Description

Brief description of the changes made in this PR.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring
  • Test addition/update
  • Updates to the dependencies has been done.

Summary by CodeRabbit

  • New Features

    • Introduced File Requirements modal accessible via "What files do I need?" link, detailing per-workflow file specifications and supplementary guidance for MaxQuant, DIA-NN, quantms, FragPipe, and mzIdentML formats.
  • Improvements

    • Updated Upload step guidance to mention FragPipe alongside existing supported workflows.
    • Enhanced modal interactions and styling for improved user experience.

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

claude and others added 2 commits January 21, 2026 12:01
Add a "What files do I need?" link in the upload section that opens
a modal explaining the required files for each supported workflow:
MaxQuant, DIA-NN, quantms, FragPipe, and mzIdentML.
…s-Dlmbv

Clarify required files for proteomics workflow uploads
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

📝 Walkthrough

Walkthrough

The change expands file guidance in the Upload step to include FragPipe workflows and introduces a new File Requirements modal providing detailed per-workflow file requirements. Modal lifecycle functions enable display and closure, with enhanced event handling for outside-click interactions across related modals.

Changes

Cohort / File(s) Summary
UI Modal Enhancement
pmultiqc_service/templates/index.html
Added expanded file upload guidance mentioning FragPipe alongside existing workflows (MaxQuant, DIA-NN, quantms, mzIdentML); introduced new File Requirements modal with per-workflow file requirements for all supported tools, supplementary files info, and workflow detection tips; implemented showFileRequirements() and closeFileRequirements() modal lifecycle functions; enhanced outside-click handling to close both Examples and File Requirements modals; improved Close button styling in Examples modal.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #269: Modifies the same template file with modal UI and outside-click event handler changes, indicating related UI infrastructure work.

Suggested labels

Review effort 3/5

Suggested reviewers

  • selvaebi

Poem

🐰 A modal appears, with files so fine,
FragPipe joins the worthy line,
Click outside to close with care,
Requirements dance through the air!

🚥 Pre-merge checks | ✅ 3
✅ 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 'Added more help to the pmultiqc service' accurately reflects the main change—adding file requirement guidance and a modal to help users understand what files are needed for different workflows.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

Copy link
Contributor

@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: 1

🤖 Fix all issues with AI agents
In `@pmultiqc_service/templates/index.html`:
- Around line 1234-1239: The UI text inside the Tip block containing "The
workflow type is automatically detected..." currently hardcodes "max 10GB" and
must reflect actual runtime limits; update this template snippet to either
display the configured upload limit (read from the same environment/config
variable used by the server, e.g. MAX_FILE_SIZE or MAX_CONTENT_LENGTH) or change
the static copy to the correct values for each deployment (use "4GB" for the
docker-compose default and "500MB" for K8s), ensuring the visible string
replacing "max 10GB" is sourced from configuration or corrected per-deployment
so the message in the div with the Tip matches actual server/nginx limits.
🧹 Nitpick comments (1)
pmultiqc_service/templates/index.html (1)

1096-1126: Modal accessibility could be improved.

The new File Requirements modal works correctly but lacks keyboard accessibility and ARIA attributes. While this is consistent with the existing Examples modal, consider these improvements for better accessibility:

  • Add role="dialog" and aria-modal="true" to the modal container
  • Add aria-labelledby pointing to the header
  • Handle Escape key to close the modal
  • Trap focus within the modal when open
♿ Optional: Add ARIA attributes for accessibility
     <!-- File Requirements Modal -->
-    <div id="fileRequirementsModal" class="modal" style="
+    <div id="fileRequirementsModal" class="modal" role="dialog" aria-modal="true" aria-labelledby="fileReqModalTitle" style="
         display: none;
         ...
     ">
         <div class="modal-content" style="...">
             <div style="display: flex; justify-content: space-between; align-items: center; margin-bottom: 20px;">
-                <h3 style="margin: 0; color: `#333`;">📋 File Requirements by Workflow</h3>
+                <h3 id="fileReqModalTitle" style="margin: 0; color: `#333`;">📋 File Requirements by Workflow</h3>

And add Escape key handling in the script:

document.addEventListener('keydown', function(event) {
    if (event.key === 'Escape') {
        closeExamples();
        closeFileRequirements();
    }
});

Comment on lines +1234 to +1239
<div style="margin-top: 20px; padding: 15px; background: #e7f3ff; border-radius: 8px; border-left: 4px solid #007bff;">
<p style="margin: 0; color: #004085; font-size: 0.9em;">
<strong>Tip:</strong> The workflow type is automatically detected based on the files in your ZIP archive.
Make sure to include all required files in a single ZIP archive (max 10GB).
</p>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for file size limit configuration in the codebase
rg -n -i "10.*gb|max.*size|upload.*limit|size.*limit" --type-add 'config:*.{py,yml,yaml,json,toml,ini,cfg,env}' --type config

Repository: bigbio/pmultiqc

Length of output: 2474


Update the 10GB limit claim to match actual deployment configuration.

The template states a 10GB maximum file size, but the production docker-compose default (MAX_CONTENT_LENGTH) is set to 4GB. Additionally, K8s deployments have a 500MB nginx limit. Users will see 10GB in the UI but may be unable to upload files beyond the actual deployment limit, causing confusion. Either:

  1. Update the template to reflect the actual limit for each deployment (4GB for docker-compose, 500MB for K8s), or
  2. Document that the limit is configurable via MAX_FILE_SIZE environment variable, or
  3. Standardize the limits across all deployment configurations.
🤖 Prompt for AI Agents
In `@pmultiqc_service/templates/index.html` around lines 1234 - 1239, The UI text
inside the Tip block containing "The workflow type is automatically detected..."
currently hardcodes "max 10GB" and must reflect actual runtime limits; update
this template snippet to either display the configured upload limit (read from
the same environment/config variable used by the server, e.g. MAX_FILE_SIZE or
MAX_CONTENT_LENGTH) or change the static copy to the correct values for each
deployment (use "4GB" for the docker-compose default and "500MB" for K8s),
ensuring the visible string replacing "max 10GB" is sourced from configuration
or corrected per-deployment so the message in the div with the Tip matches
actual server/nginx limits.

@ypriverol ypriverol merged commit f138358 into main Jan 21, 2026
24 checks passed
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.

2 participants