Feature/recent networks

chrtannus commented 2 years ago

General information

Associated issues: #133

Checklist

Author:

[X] One or more reviewers have been assigned.
[x] Automated tests have been included in this pull request, if possible, for the new feature(s) or bug fix.
[X] The associated GitHub issues are included (above).
[X] Notes have been included (below).

Reviewers:

[x] All automated checks are passing (green check next to latest commit).
[x] At least one reviewer has signed off on the pull request. Reviewers have two business days to review the pull request, after which the author may merge in the pull request unilaterally.

Notes

This adds a "Recent Networks" section to the homepage, which is totally hidden if there are no items in the browser's local storage.

Each recent network item provides an overflow menu with the following actions:
- Remove [this network]
- Open in New Tab
By clicking the network thumbnail you open the network in the same tab.
Right now, the maximum number of recent networks that can be displayed is 25, though there's no current limit to the number of items in the local store.
Each network's metadata is saved in the local storage (using the LocalForage library), where the network id is the key and the value object has the following fields:
- secret
- name
- thumbnail: PNG image as a base64 string
- opened: the date/time (in milliseconds) the network was last opened on the browser
- modified: the date/time (in milliseconds) the network was modified through the same browser
These Cytoscape events trigger an update to the network metadata in the Local Storage: 'data add remove move layoutstop viewport'
- NOTE: using a debounce function to prevent unnecessary updates

mikekucera commented 2 years ago

I found what might be considered a bug...

When you make a change to the network there is a 3 second delay before the changes are synched to the server. So if you make a change, for example set all nodes to red, then click the "home" button in the top left corner before the 3 second delay fires, the changes do not get sent to the server, but the local storage thumbnail shows the nodes being all red, and when you click on it the network shows up without the red nodes.

Perhaps we need a mechanism to automatically trigger the server sync when the user navigates away from the page. Because the 3 second delay can't always be relied upon.

mikekucera commented 2 years ago

The NDEX page...

I think the Search NDEx tab should be first, because if the Browse My Networks tab is empty it just looks like an empty dialog.
If the search returns nothing there should be a message like "search did not find any results" or something.

mikekucera commented 2 years ago

Got this exception trying to import the galExpData.csv file from the cytoscape desktop sampleData folder. This file is only 416kb, and its the same network we use for the "try a sample network" link, so I don't think it should fail.

PayloadTooLargeError: request entity too large
    at readStream (/Users/mkucera/git/cytoscape-explore/node_modules/raw-body/index.js:155:17)
    at getRawBody (/Users/mkucera/git/cytoscape-explore/node_modules/raw-body/index.js:108:12)
    at read (/Users/mkucera/git/cytoscape-explore/node_modules/body-parser/lib/read.js:77:3)
    at jsonParser (/Users/mkucera/git/cytoscape-explore/node_modules/body-parser/lib/types/json.js:135:5)
    at Layer.handle [as handle_request] (/Users/mkucera/git/cytoscape-explore/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/Users/mkucera/git/cytoscape-explore/node_modules/express/lib/router/index.js:317:13)
    at /Users/mkucera/git/cytoscape-explore/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/Users/mkucera/git/cytoscape-explore/node_modules/express/lib/router/index.js:335:12)
    at next (/Users/mkucera/git/cytoscape-explore/node_modules/express/lib/router/index.js:275:10)
    at logger (/Users/mkucera/git/cytoscape-explore/node_modules/morgan/index.js:144:5)

mikekucera commented 2 years ago

I also want to state my concerns with the Excel import preview page. I've already discussed this with Christian, and I don't think this should block this PR, its just my opinion, and user testing still needs to be done, but....

I really don't like the preview page. I think It's trying to relay some very simple information, but in a way that obfuscates it.

In my opinion if we just displayed the information in text it would be much clearer. For example:

Edge Table Import Preview Total 350 Rows to be imported Source Node Attribute: Column1 Target Node Attribute: Column2 Additional Edge Attributes: Column3, Column4, Column5

That's all the information we are trying to relay to the user. See how simple it is when just written out?

The preview page is way too complicated... with its blue nodes, floating labels, single-row horizontally scrolling table in a bubble, and weird mini histograms. I think it will be very confusing for users.

That's my 2 cents.

maxkfranz commented 2 years ago

Got this exception trying to import the galExpData.csv file from the cytoscape desktop sampleData folder. This file is only 416kb, and its the same network we use for the "try a sample network" link, so I don't think it should fail.

See UPLOAD_LIMIT https://github.com/cytoscape/cytoscape-explore/blob/dev/.env#L8

@d2fong, @jingjingbic, is there an issue w.r.t. CX?

maxkfranz commented 2 years ago

I also want to state my concerns with the Excel import preview page.

Good observations

There are at least three aims of a preview for the edges case. The user can easily confirm that:

(1) The edges are right (i.e. topology).

Each edge connects to the right nodes.
Each edge is in the right direction, if applicable.

(2) The edge data are right.

Each edge has all of its data attributes.
Each data attribute has the right value.

(3) Nothing was missed.

This is similar to (1) and (2), but it's qualitatively distinct.
All of the edges were included. None were missed or included erroneously (misinterpreted header, a (semi-)empty row doesn't create something weird).
All of the attribute data for all edges were included. Nothing was missed, and problems are obvious (e.g. a node that is missing a value).

You can imagine similar aims for nodes. Each aim can be hit by presenting information to the user by example or in aggregate. @mikekucera's example presents everything in aggregate with text preview. The current approach uses a mix of example and aggregate visual preview.

Whatever we use as a preview should hit the aims -- at least the ones above, possibly more -- and we should aim for simplicity. If we have a simpler preview than what we have now that hits the aims, then that's great.

@mikekucera, @chrtannus, do you want to spin this out into a preview discussion since it should probably live longer and outside this PR?

d2fong commented 2 years ago

Got this exception trying to import the galExpData.csv file from the cytoscape desktop sampleData folder. This file is only 416kb, and its the same network we use for the "try a sample network" link, so I don't think it should fail.

See UPLOAD_LIMIT https://github.com/cytoscape/cytoscape-explore/blob/dev/.env#L8

@d2fong, @jingjingbic, is there an issue w.r.t. CX?

What is the context of this upload limit config value? I don't know if there would be an issue without more info.

chrtannus commented 2 years ago

I found what might be considered a bug...

When you make a change to the network there is a 3 second delay before the changes are synched to the server. So if you make a change, for example set all nodes to red, then click the "home" button in the top left corner before the 3 second delay fires, the changes do not get sent to the server, but the local storage thumbnail shows the nodes being all red, and when you click on it the network shows up without the red nodes.

Perhaps we need a mechanism to automatically trigger the server sync when the user navigates away from the page. Because the 3 second delay can't always be relied upon.

Yes, I've noticed this issue. But should we fix it here or create a new ticket, since this is independent from the Recent Networks feature, though it's made more apparent now?

mikekucera commented 2 years ago

I think it’s fine to consider this a separate bug.

On Mon, Mar 7, 2022 at 12:17 PM Christian Lopes @.***> wrote:

I found what might be considered a bug...

When you make a change to the network there is a 3 second delay before the changes are synched to the server. So if you make a change, for example set all nodes to red, then click the "home" button in the top left corner before the 3 second delay fires, the changes do not get sent to the server, but the local storage thumbnail shows the nodes being all red, and when you click on it the network shows up without the red nodes.

Perhaps we need a mechanism to automatically trigger the server sync when the user navigates away from the page. Because the 3 second delay can't always be relied upon.

Yes, I've noticed this issue. But should we fix it here or create a new ticket, since this is independent from the Recent Networks feature, though it's made more apparent now?

— Reply to this email directly, view it on GitHub https://github.com/cytoscape/cytoscape-explore/pull/142#issuecomment-1060928054, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABI2JAD3GPSOSTQNMZLMTALU6Y2ZJANCNFSM5P3WSSBA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

chrtannus commented 2 years ago

@mikekucera It seems we can handle the other issues in separate tickets. Does it make sense?

mikekucera commented 2 years ago

Yes

On Mon, Mar 7, 2022 at 12:20 PM Christian Lopes @.***> wrote:

@mikekucera https://github.com/mikekucera It seems we can handle the other issues in separate tickets. Does it make sense?

— Reply to this email directly, view it on GitHub https://github.com/cytoscape/cytoscape-explore/pull/142#issuecomment-1060931181, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABI2JACWH4PWI7I5SM6XZ4TU6Y3ETANCNFSM5P3WSSBA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

maxkfranz commented 2 years ago

For new issues & discussions see:

149
150
144

cytoscape / cytoscape-explore

Feature/recent networks #142

149

150

144