Skip to content

Feat delete#96

Merged
tonywu1999 merged 7 commits intodevelfrom
feat-delete
Apr 16, 2026
Merged

Feat delete#96
tonywu1999 merged 7 commits intodevelfrom
feat-delete

Conversation

@tonywu1999
Copy link
Copy Markdown
Contributor

@tonywu1999 tonywu1999 commented Apr 16, 2026

Motivation and Context

The PR adds interactive edge deletion functionality to the Cytoscape network visualization widget. Users can now remove edges (connections between proteins) directly from the network by right-clicking or using Ctrl+Click, enabling dynamic network refinement and exploration. The feature maintains backward compatibility by keeping plain left-click behavior for opening evidence links.

Solution Summary

The implementation introduces a deleteEdge() helper function in the JavaScript widget that removes edges from the graph and notifies the R/Shiny server of deletions. The event handler logic was refactored to support both deletion (right-click/Ctrl+Click) and navigation (plain click) actions on edges, with the "prevent default" behavior removed as indicated in the commit message.

Detailed Changes

  • Updated legend/help text with a new section instructing users: "Right-click or Ctrl+Click an edge to remove it from the network"
  • Added deleteEdge(edge) helper function that:
    • Sends a Shiny input event named ${el.id}_edge_deleted containing the deleted edge's source, target, and interaction properties when Shiny is available
    • Removes the edge from the Cytoscape graph instance
    • Uses priority "event" for immediate Shiny notification
  • Refactored edge event handler from single tap handler to combined "cxttap tap" handler:
    • Right-click (cxttap event) or Ctrl+Click triggers deleteEdge()
    • Plain left-click continues to open evidence links and sends ${el.id}_edge_clicked event
    • PTM attachment edges are exempt from deletion to preserve visualization integrity
  • Removed preventDefault() logic from edge interactions as per the commit message

Testing

No new unit tests have been added or modified to verify the edge deletion functionality. The existing test file tests/testthat/test-utils_cytoscapeNetwork.R does not include tests for the JavaScript widget's interactive features or the new deletion capability.

Coding Guidelines

The implementation adheres to the existing code standards:

  • JavaScript code follows established formatting conventions with proper indentation and clear comments
  • Helper function includes descriptive comments explaining the deletion and notification process
  • Appropriate conditional checks for Shiny availability before attempting event notifications
  • Guard clause to prevent deletion of PTM attachment edges

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 16, 2026

Warning

Rate limit exceeded

@tonywu1999 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 42 minutes and 27 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 27 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 08f19fab-9c75-43f4-b624-b3ec5eb9f90f

📥 Commits

Reviewing files that changed from the base of the PR and between baae9d0 and ca1f975.

📒 Files selected for processing (1)
  • inst/htmlwidgets/cytoscapeNetwork.js
📝 Walkthrough

Walkthrough

Updated the Cytoscape network widget's JavaScript to add edge deletion functionality via right-click or Ctrl+Click. Introduced a deleteEdge() helper that notifies Shiny when edges are deleted and removes them from the graph. Modified the edge event handler to distinguish between deletion (right-click/Ctrl+Click) and evidence link opening (plain click). Updated help text accordingly.

Changes

Cohort / File(s) Summary
Edge Deletion Feature
inst/htmlwidgets/cytoscapeNetwork.js
Added deleteEdge() helper function that sends Shiny input event ${el.id}_edge_deleted with edge metadata and removes the edge from the graph. Modified edge event handler from tap to combined "cxttap tap" handler to support both deletion (right-click/Ctrl+Click) and evidence link opening (plain click). Updated legend text to document new deletion capability.

Sequence Diagram

sequenceDiagram
    participant User
    participant Cytoscape as Cytoscape<br/>(Frontend)
    participant Shiny as Shiny<br/>(Backend)
    
    alt Right-Click or Ctrl+Click
        User->>Cytoscape: cxttap event on edge
        Cytoscape->>Cytoscape: deleteEdge(edge)
        Cytoscape->>Shiny: Send input event<br/>(edge_deleted)
        Shiny->>Shiny: Process deletion event
        Cytoscape->>Cytoscape: Remove edge from graph
    else Plain Click
        User->>Cytoscape: tap event on edge
        Cytoscape->>Shiny: Send input event<br/>(edge_clicked)
        Cytoscape->>User: Open evidence link
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 A right-click whisper, edges do part,
Ctrl+Click magic from the start,
Shiny receives the deletion's call,
While clicks still open links so tall! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Feat delete' is vague and generic; it lacks specificity about what feature is being added or what is being deleted. Use a more descriptive title like 'Add edge deletion feature with right-click and Ctrl+Click support' to clearly convey the main change.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check ✅ Passed The PR description is comprehensive and well-structured, providing detailed motivation, solution summary, implementation details, and testing information that align with the template requirements.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-delete

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
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: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@inst/htmlwidgets/cytoscapeNetwork.js`:
- Around line 459-462: The delete-edge interaction path currently calls
deleteEdge(edge) and returns without updating the legend; add a call to
buildLegend() immediately after deleteEdge(edge) (before the return) so the
legend is rebuilt from the updated cy.edges() when an edge is removed (same
branch handling evt.type === "cxttap" or evt.originalEvent.ctrlKey). Ensure you
reference the existing deleteEdge and buildLegend functions so the legend
refresh happens as part of the same interaction flow.
- Around line 454-461: The current combined event handler (cy.on("cxttap tap",
"edge", ...)) returns early for edges with edge.data("edge_type") ===
"ptm_attachment", preventing the delete branch from ever running; remove that
early return and instead only skip PTM edges in the evidence-link-specific code
path so ptm_attachment edges can still be deleted via the Ctrl+Click/right-click
branch (keep references to the existing cy.on handler and deleteEdge(edge) call
and move the ptm_attachment check into the evidence-link/evidence-link-handling
block).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1452d959-8410-475d-871e-1ea19cd48597

📥 Commits

Reviewing files that changed from the base of the PR and between a26e6fb and baae9d0.

📒 Files selected for processing (1)
  • inst/htmlwidgets/cytoscapeNetwork.js

Comment thread inst/htmlwidgets/cytoscapeNetwork.js
Comment thread inst/htmlwidgets/cytoscapeNetwork.js
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.79%. Comparing base (a26e6fb) to head (ca1f975).

Additional details and impacted files
@@           Coverage Diff           @@
##            devel      #96   +/-   ##
=======================================
  Coverage   76.79%   76.79%           
=======================================
  Files           8        8           
  Lines        1030     1030           
=======================================
  Hits          791      791           
  Misses        239      239           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tonywu1999 tonywu1999 merged commit 3a8dadd into devel Apr 16, 2026
4 checks passed
@tonywu1999 tonywu1999 deleted the feat-delete branch April 16, 2026 14:32
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