Skip to content
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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

melvin15may
Copy link
Contributor

Fixes issue #262
Added save button with its own dedicated modal.

Screenshot of layout editor before this PR
BEFORE

Screenshot of layout editor after this PR
AFTER

Screenshot of Save dialog
Save dialog modal

@@ -584,7 +594,7 @@ var graphPage = {
graphPage.cyGraph.layout(layout);

},
saveLayout: function (layoutName) {
saveLayout: function (layoutName, modalNameId) {
Copy link
Collaborator

@adbharadwaj adbharadwaj May 31, 2017

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.

Copy link
Contributor Author

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.

@jlaw9
Copy link
Collaborator

jlaw9 commented Aug 22, 2017

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"

@tmmurali
Copy link
Member

tmmurali commented Aug 22, 2017 via email

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.

4 participants