-
Notifications
You must be signed in to change notification settings - Fork 41
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
Fixes issue #262 added Save button to graph layout editor #271
base: develop
Are you sure you want to change the base?
Conversation
static/js/graphs_page.js
Outdated
@@ -584,7 +594,7 @@ var graphPage = { | |||
graphPage.cyGraph.layout(layout); | |||
|
|||
}, | |||
saveLayout: function (layoutName) { | |||
saveLayout: function (layoutName, modalNameId) { |
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.
I dont like the change to saveLayout
method. It doesn't make sense in terms of the method signature. Here are few questions for you -
- Can we rename modalNameId to a more intuitive name?
- Can we move modalNameID out of saveLayout method? It seems that we need it only for closing the modal.
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.
I can rename modalNameId
to something more intuitive. Any suggestions?
I need the ID of the modal element to toggle between the element's state only on successful callback. If I take it out will have to implement a callback (callback from a callback situation). I thought of making it modular, in case we use this function for other DOM elements.
changed saveLayout parameter name
The current title of "Save Layout" leads users to believe they are saving the layout of the nodes (x and y positions) rather than the x and y positions and the style of every node which leads to issues like #310. I suggest we change the save popup title to "Save Layout and Style" or add a description in the popup saying something like "Current x and y positions of nodes as well as style attributes of every node and edge will be saved" |
I suggest "Save Positions and Style" and to change the voice of the pop-up
to active: "Save the current x- and y-coordinates of every node and the
style attributes of every node and edge."
…On Tue, Aug 22, 2017 at 9:21 AM, Jeff Law ***@***.***> wrote:
The current title of "Save Layout" leads users to believe they are saving
the layout of the nodes (x and y positions) rather than the x and y
positions and the style of every node which leads to issues like #310
<#310>. I suggest we
change the save popup title to "Save Layout and Style" or add a description
in the popup saying something like "Current x and y positions of nodes as
well as style attributes of every node and edge will be saved"
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#271 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGkWUJ3EHzjkF5RWSjXD64wzaAdrpwILks5satXggaJpZM4NsKob>
.
|
Fixes issue #262
Added save button with its own dedicated modal.
Screenshot of layout editor before this PR

Screenshot of layout editor after this PR

Screenshot of Save dialog
