Skip to content
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

Show all survival attributes and remove duplicates in study clinical data tab #4883

Merged
merged 5 commits into from
May 14, 2024

Conversation

gblaih
Copy link
Contributor

@gblaih gblaih commented Mar 29, 2024

Fix cBioPortal/cbioportal#10696

Continue to hide survival attributes (besides OS_STATUS) in Summary tab.

Copy link

netlify bot commented Mar 29, 2024

Deploy Preview for cbioportalfrontend ready!

Name Link
🔨 Latest commit 65b0d1b
🔍 Latest deploy log https://app.netlify.com/sites/cbioportalfrontend/deploys/663d42cc823e19000894711e
😎 Deploy Preview https://deploy-preview-4883--cbioportalfrontend.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@inodb inodb added the bug label Apr 1, 2024
@computed get chartMetaSetForClinicalAttributes(): {
[id: string]: ChartMeta;
} {
let _chartMetaSet = _.fromPairs(this._customCharts.toJSON());
Copy link
Collaborator

Choose a reason for hiding this comment

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

i don't think we need the leading underscore here

} = {};
return _.reduce(
Array.from(this._chartVisibility.entries() || []),
(acc, [chartUniqueKey, visible]) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

put the type in the callback definition instead of down below where you pass the argument. This will prevent you from using 'as' casting which loosens typing.

_.reduce(acc:ChartMeta[], ...) =>

@gblaih gblaih force-pushed the study-clinical-data-columns branch from 092b417 to 9a21ff7 Compare April 26, 2024 22:04
@@ -2895,9 +2900,19 @@ export function getChartSettingsMap(
}
chartSettingsMap[id] = chartSetting;
});
// attributes disabled on the summary tab (used in Clinical Data tab but not Summary tab like most survival attributes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

good comment!

if (layout.i && chartSettingsMap[layout.i]) {
if (
layout.i &&
chartSettingsMap[layout.i] &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

would it be possible to avoid putting these disabled attributes into chartSettingsMap in the first place so we wouldn't have to check this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we still want to keep the disabled attributes in the map for user settings purposes for Clinical Data tab columns, just without the layout or else it will mess with chart layout on summary tab.

@gblaih gblaih force-pushed the study-clinical-data-columns branch from 4182760 to 65b0d1b Compare May 9, 2024 21:40
@alisman alisman merged commit 911ae1e into cBioPortal:master May 14, 2024
13 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Studyview clinical data tab lists Overall Survival column twice, but not OS_MONTHS column
3 participants