-
Notifications
You must be signed in to change notification settings - Fork 0
parental line modal: update link and button styling #358
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
Conversation
✅ Deploy Preview for cell-catalog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| }} | ||
| href="https://www.allencell.org/cell-catalog.html" | ||
| href={`/cell-line/AICS-${props.cellLineId}-${props.cloneNumber}/`} | ||
| target="_blank" |
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.
keeping target="_blank" so the parental line opens in a new tab. Open to feedback if internal navigation would be better in the same tab
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.
we can double check with travis
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.
nit: I'm pretty sure we have a utility function that takes an id and makes it AICS-CELL_ID. you could adjust it to add clone number optionally
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.
yeah its formatCellLineId
interim17
left a comment
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.
Just mentioned the util you could use for the cell line string
| }} | ||
| href="https://www.allencell.org/cell-catalog.html" | ||
| href={`/cell-line/AICS-${props.cellLineId}-${props.cloneNumber}/`} | ||
| target="_blank" |
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.
yeah its formatCellLineId
src/utils/index.ts
Outdated
| if (cloneNumber !== undefined) { | ||
| return `AICS-${cellLineId}-${cloneNumber}`; | ||
| } | ||
|
|
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.
The cell line link currently uses non-zero-padded version of ids, e.g.(/cell-line/AICS-5-50/ or /disease-cell-line/AICS-97/)
|
@meganrm @interim17 Just wanted to re-request your review to see if the update in the util looks okay. To make the links work, the function now returns both padded and non-padded formats (0005 vs 5). |
Problem
What is the problem this work solves, including
closes #354
closes #355
Solution
What I/we did to solve this problem
go to parental linebutton now directs to the corresponding cell line pageType of change
Please delete options that are not relevant.
Steps to Verify:
go to parental linebutton