Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#8053 closed enhancement (duplicate)

graph editor minor bugs and improvements

Reported by: rkirov Owned by: was
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: notebook Keywords:
Cc: mpatel Merged in:
Authors: rkirov Reviewers: Mitesh Patel
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

There was bug in the graph_editor which counted the release of a dragging move as a beginning of a double click. This should reduce the number of accidental node deletions.

Also added the following improvements.

  • there is one-step undo available.
  • a node dragged out of the iframe returns to its original position. Deletion is preformed only if mouse is released between the canvas and the iframe.
  • live sliders only shown when live is enabled.
  • live algorithm never pushes nodes out of bounds.

Note that JSbeautifier.com moved some if else statements indents which is reflected in the patch (even though there was not actual code change in those parts).

Attachments (1)

8053.patch (17.1 KB) - added by rkirov 11 years ago.

Download all attachments as: .zip

Change History (6)

Changed 11 years ago by rkirov

comment:1 Changed 11 years ago by rkirov

  • Status changed from new to needs_review

comment:2 Changed 11 years ago by mpatel

  • Reviewers set to Mitesh Patel

The patch looks good. The new features and fixes are great!

  • I recommend running graph_editor.js through JSLint on its "The Good Parts" setting. (Feel free to set white: true at the top of the file, since the beautifier now takes care of that.). It found:
    • Missing ; at the end of line 6.
    • removed_node is not defined. I suggest adding it to line 6.
  • Do people find it useful to turn off live mode but still be able to adjust the sliders? I often do this to "freeze" movement while I make adjustments, but I'm definitely not a typical user.
  • Until they read the help text, users may expect the "Undo" button to undo the last action. Can you extend its capability? Or should we call it "Undelete" and restore a deleted edge, as well?

comment:3 Changed 11 years ago by rkirov

  • Status changed from needs_review to needs_work

thanks, mitesh I will make a few more edits and submit a new patch.

comment:4 Changed 11 years ago by rkirov

  • Resolution set to duplicate
  • Status changed from needs_work to closed

merged with ticket 8222

comment:5 Changed 11 years ago by mvngu

  • Milestone changed from sage-4.4 to sage-duplicate/invalid/wontfix
Note: See TracTickets for help on using tickets.