search

VEuPathDB / web-monorepo

A monorepo that contains all frontend code for VEuPathDB websites
Apache License 2.0
2 stars 0 forks source link

OrthoMCL - updates to protein mapping tool #1215

Closed dmfalke closed 1 month ago

dmfalke commented 2 months ago

Fixes #1213

bobular commented 2 months ago

Ready for review. Maybe add an "also" into the drag-and-drop guidance? ("You can also drag...")

bobular commented 2 months ago

Might need to make a new PR so you can review it? Or we can do a quick sync review and I can review/approve, I guess?

bobular commented 1 month ago

Hi @dmfalke - this is hopefully all done now. I tested the individual results page with an expired job and now it doesn't keep polling or trying to rerun!

dmfalke commented 1 month ago

Hi @dmfalke - this is hopefully all done now. I tested the individual results page with an expired job and now it doesn't keep polling or trying to rerun!

Great! I'm taking a look now.

dmfalke commented 1 month ago

Regarding the latest change, I'm not sure about the Page not found heading. I think a better heading would be Diamond job - expired.

I also noticed that there is a moment where the page heading says Blast job - pending. I think this is an intermediate component that needs to use the configured workspace title, or something like that.

Finally, the last request that hasn't been addressed is to add the number of rows in the preview table. It should say something like:

Showing the first 100 sequences in your query file.

If the number of number of rows is less than 100, it can say

Showing all 48 sequences in your query file.

This can replace the existing text of Mapped proteins.

bobular commented 1 month ago

Thanks @dmfalke - all done except for the "Blast job - pending" verbiage. This is a bit trickier than it looks. I'll think about it tomorrow.

dmfalke commented 1 month ago

Thanks @dmfalke - all done except for the "Blast job - pending" verbiage. This is a bit trickier than it looks. I'll think about it tomorrow.

Take a look at the following to see how we are passing a configurable header value to other components: https://github.com/VEuPathDB/web-monorepo/blob/89b02fe70dea193addccef5d21b5961854507b67/packages/libs/multi-blast/src/lib/controllers/BlastWorkspaceRouter.tsx#L27

bobular commented 1 month ago

Thanks @dmfalke - all done except for the "Blast job - pending" verbiage. This is a bit trickier than it looks. I'll think about it tomorrow.

Take a look at the following to see how we are passing a configurable header value to other components:

https://github.com/VEuPathDB/web-monorepo/blob/89b02fe70dea193addccef5d21b5961854507b67/packages/libs/multi-blast/src/lib/controllers/BlastWorkspaceRouter.tsx#L27

Yeah I've seen that, but what we provide to that prop from proteinMappingRouter is rather long-winded.

dmfalke commented 1 month ago

I've reviewed and tested this, using both ortho and plasmo sites. It all looks great to me!