Skip to content

SC fixes #2763

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

Merged
merged 12 commits into from
Feb 7, 2025
Merged

SC fixes #2763

merged 12 commits into from
Feb 7, 2025

Conversation

airenzp
Copy link
Collaborator

@airenzp airenzp commented Feb 7, 2025

Description

Do not preselect a sample but show only Samples tabs until a sample is selected. Improved sample heading. Aligned legend with plot for GDC. Added subtype in BALL sjpp. I used the idea about hiding other tabs that we discussed @siosonel to do as GDC asked.

Screenshot 2025-02-07 at 12 18 17 PM

Two plots in a row with the legend besides the plot:

Screenshot 2025-02-07 at 1 36 25 PM

Checklist

Check each task that has been performed or verified to be not applicable.

  • Tests: added and/or passed unit and integration tests, or N/A
  • Todos: commented or documented, or N/A
  • Notable Changes: updated release.txt, prefixed a commit message with "fix:" or "feat:", added to an internal tracking document, or N/A

@airenzp airenzp requested a review from xzhou82 February 7, 2025 16:19
@xzhou82 xzhou82 requested a review from compbiolover February 7, 2025 18:04
@airenzp airenzp changed the title In the SC, showed samples table prompt aligned with sample info SC fixes Feb 7, 2025
Copy link
Contributor

@compbiolover compbiolover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good. However, I noticed a layout issue: When viewing the UMAP and t-SNE plots side by side, then switching to the differential expression tab to run an analysis, the output table only takes up half of the sandbox width instead of the full width like the samples table does. While there's a resize handle in the corner of the table, it's not functional when dragged.

@airenzp
Copy link
Collaborator Author

airenzp commented Feb 7, 2025

Overall this looks good. However, I noticed a layout issue: When viewing the UMAP and t-SNE plots side by side, then switching to the differential expression tab to run an analysis, the output table only takes up half of the sandbox width instead of the full width like the samples table does. While there's a resize handle in the corner of the table, it's not functional when dragged.

Thanks Andrew. I made the DE table resizable now. Also I have reduced the table heights for the GDC Frontend Framework as they add one extra heading and the long height on the tables makes them scroll

@compbiolover
Copy link
Contributor

Overall this looks good. However, I noticed a layout issue: When viewing the UMAP and t-SNE plots side by side, then switching to the differential expression tab to run an analysis, the output table only takes up half of the sandbox width instead of the full width like the samples table does. While there's a resize handle in the corner of the table, it's not functional when dragged.

Thanks Andrew. I made the DE table resizable now. Also I have reduced the table heights for the GDC Frontend Framework as they add one extra heading and the long height on the tables makes them scroll

You're welcome. The table can now be resized to be smaller or larger. I didn't realize there was a limit on the width of the DE table. At about ~half way across the page the table can't be made larger. My previous comment can be discarded as I thought the DE table could be expanded to fill the full width of the page but I see that isn't the case

@airenzp
Copy link
Collaborator Author

airenzp commented Feb 7, 2025

Thanks for the scrolling @siosonel . Please see my last commit with this link where a sample is preselected. It works if I add the state check. Also I had to add a fix in the tabs so on init it reads if the tab is visible.

…rsor in toggleButtons render(); fix failing test
Copy link
Collaborator Author

@airenzp airenzp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks @siosonel

@siosonel siosonel merged commit 83b961f into master Feb 7, 2025
3 checks passed
@siosonel siosonel deleted the sc-samples-tab-fix branch February 7, 2025 21:34
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