-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Fix gene card link formatter for datavzrd report and update linkout styles #23
Conversation
WalkthroughThe changes involve a comprehensive update to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
workflow/scripts/gene_card_link_formatter.js (2)
1-3
: Consider adding input validation
The function assumes value
will be a valid string containing comma-separated genes. Consider adding validation to handle edge cases like null, undefined, or malformed input.
function gene_card_link_formatter(value, row) {
+ if (!value) return '';
let genes = value.split(",");
+ genes = genes.filter(g => g.trim()); // Remove empty entries
let rows = "";
12-23
: Enhance accessibility and usability
- The fixed max-height (30vh) might truncate content on smaller screens
- Missing ARIA attributes for better screen reader support
- Consider adding a caption or aria-label for the table
- let table = `<div style="overflow-y: auto; max-height: 30vh; min-width: 140px;">
- <table style="border-collapse: collapse; width: 100%; box-shadow: 0 2px 10px rgba(0, 0, 0, 0.1);">
+ let table = `<div style="overflow-y: auto; max-height: max(30vh, 200px); min-width: 140px;">
+ <table style="border-collapse: collapse; width: 100%; box-shadow: 0 2px 10px rgba(0, 0, 0, 0.1);"
+ aria-label="Gene Cards References">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- workflow/scripts/gene_card_link_formatter.js (1 hunks)
🔇 Additional comments (1)
workflow/scripts/gene_card_link_formatter.js (1)
30-30
: LGTM!
The return statement is clean and consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Many thanks for this cleanup!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- workflow/scripts/gene_card_link_formatter.js (1 hunks)
🔇 Additional comments (1)
workflow/scripts/gene_card_link_formatter.js (1)
1-4
: LGTM: Clean function structure and initialization
The function signature and initial setup are well-structured for handling multiple genes.
This PR fixes an issue with jQuery not being accessible in the function by removing
which was unnecessary anyways.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor