dondi / GRNsight

Web app and service for modeling and visualizing gene regulatory networks.
http://dondi.github.io/GRNsight
BSD 3-Clause "New" or "Revised" License
17 stars 8 forks source link

Disabling node coloring will be enabled upon reload #943

Closed ahmad00m closed 2 years ago

ahmad00m commented 2 years ago

The settings should remain as it is when reload bottom is used. However, The node coloring will be enabled upon reload. Also, any other setting that changes upon reload needs to be fixed so it doesn't change after reload. Moreover, another feature can be created to reset the setting/graph together.

dondi commented 2 years ago

This bug will be helpful in getting acquainted with the overall application code structure. Start by tracking down the code for the Enable Node Coloring checkbox and look at how it interacts with the grnState variable. Then look at the Reset command and track down how it appears to be resetting

dondi commented 2 years ago

A library that may be useful to know about in the background is jQuery

Sarronnn commented 2 years ago

Could not get the graph to show up: do I need to upload a gene sequence?

Looked at developers tool: Error message showed "Failed to load resource: the server responded with a status of 404 (Not Found)" " DevTools failed to load source-ma

kdahlquist commented 2 years ago

You should be able to reproduce this by using a demo file under the demo menu.

Sarronnn commented 2 years ago

I tried the files under demo:

Sarronnn commented 2 years ago

Looked at graph.js and grn.state.js to locate the bug. Moved on to look at update-app.js.

dondi commented 2 years ago

@Sarronnn are there any updates on learning about this bug?

Sarronnn commented 2 years ago

if (hasExpressionData(grnState.workbook.expression)) { resetDatasetDropdownMenus(grnState.workbook);

        grnState.nodeColoring.nodeColoringEnabled = false; //was true before

Update: Change did not work Currently: Still looking at the code in detail for possible bug defects. Question: As I make the edit on my editor, and save the changes, I am able to run the local version and see those changes right?

dondi commented 2 years ago

In answer to the question, yes, saving changes in the editor should make npm run start-dev refresh (actually you should also to see the refresh taking place in the command line), but frequently you need to reload the browser window so that the new code gets loaded into the browser

For pursuing whether a possible fix is in the direction, you’ll want to make the code’s execution more “visible” to you. There are two broad options to do this:

This way you can get a finer view of what is happening internally and eventually home in on the potential issue. We can go over this live at some future time

dondi commented 2 years ago

Before we meet, the first thing to get used to is executing your local copy with the developer tools open and seeing if you can get console logs to appear after you modify your local code with them

Sarronnn commented 2 years ago

Screenshot (7)

dondi commented 2 years ago

Yes, when quotes are not used, the expectation is that there is a variable with that name, and there isn’t any at that point thus you get the error

The error that goes with print has a couple of reasons: first, JavaScript does not use print (it is console.log instead) and second, parentheses are always required with function calls

Now that you have the right form worked out, put those statements right in the code then reload the app to see if they come out while the app is being used

Sarronnn commented 2 years ago

Got it, thank you for the feedback! Update:

dondi commented 2 years ago

We answered @Sarronnn’s question on a reload function by using Inspect in the browser on the Reload item and noting its ID which was #reload. We then searched the code for that and found what appears to be the starting point of the reload sequence: https://github.com/dondi/GRNsight/blob/master/web-client/public/js/setup-load-and-import-handlers.js

@Sarronnn will start using temporary console.log technique for following the code from there and will see if she can spot the places where values are being adjusted

dondi commented 2 years ago

Upon further inspection, we found a constant called NODE_COLORING_TOGGLE_SIDEBAR which appears to represent the checkbox in the sidebar, and its value appears to be hardcoded to true: https://github.com/dondi/GRNsight/blob/master/web-client/public/js/update-app.js#L802

Sarronnn commented 2 years ago

Screenshot (64) Question

dondi commented 2 years ago

In answer to the comment above, it looks like applying a pattern like this will address the issue:

if (grnState.nodeColoring.nodeColoringEnabled) {
    $(NODE_COLORING_TOGGLE_SIDEBAR).prop("checked", "checked");
} else {
    $(NODE_COLORING_TOGGLE_SIDEBAR).removeProp("checked");
}

Try that out and see if it has the desired effect; also search the code for NODE_COLORING_TOGGLE_SIDEBAR in case this selector is being used in other parts of the code, as well as nodeColoringEnabled to see what other parts of the code are using this grnState property

When you get this to work, let’s aim to have you issue a pull request so we can this fix onto beta (and eventually onto v6.0.0)

Sarronnn commented 2 years ago

Went to ask Prof.Dondi for help:

dondi commented 2 years ago

Some listing errors noted; just trailing spaces on five lines. @Sarronnn will fix this then push, then upon getting the green check, will open a pull request that merges into beta (not master yet)

dondi commented 2 years ago

Linting fixes committed with https://github.com/dondi/GRNsight/commit/840e26d15f434933b2c639140b086fe1449d4d41