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

Hide download controls #3716

Merged
merged 2 commits into from
May 24, 2022
Merged

Conversation

pvannierop
Copy link
Contributor

@pvannierop pvannierop commented Apr 20, 2021

Hide Download Controls

Feature description

This PR will hide all download and copy-to-clipboard options from cBioPortal. This behavior is controlled by a new skin_hide_download_controls property in serverConfig. The removed download options include:

  • Download tab in Studies Overview page.
  • Download tab in Results View.
  • Download action button in Study View.
  • Download options in Charts and Tables.

Tests

Localdb-e2e tests are added that check occurrence of download/clipboard icons and of occurrence of the word download anywhere on the page. Also, test that check for new tabs appearing on Studies Overview, Results View, Study View, Patient View and Group Comparison are included (to warn the developer that the test coverage should be increased to cover the new tab).

src/pages/groupComparison/ClinicalDataEnrichmentsTable.tsx Outdated Show resolved Hide resolved
@@ -68,7 +69,10 @@ export default class PatientHeader extends React.Component<
<h5>{patient.id}</h5>
<ClinicalInformationPatientTable
showFilter={false}
showCopyDownload={false}
showCopyDownload={
false &&
Copy link
Contributor

Choose a reason for hiding this comment

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

false && anything === false

src/pages/patientView/patientHeader/SampleInline.tsx Outdated Show resolved Hide resolved
src/pages/patientView/trialMatch/TrialMatchTable.tsx Outdated Show resolved Hide resolved
src/pages/resultsView/ResultsViewPage.tsx Outdated Show resolved Hide resolved
src/shared/components/proteinChainPanel/PdbChainTable.tsx Outdated Show resolved Hide resolved
src/shared/components/query/GenesetsVolcanoSelector.tsx Outdated Show resolved Hide resolved
src/shared/components/query/GisticGeneSelector.tsx Outdated Show resolved Hide resolved
src/shared/components/query/MutSigGeneSelector.tsx Outdated Show resolved Hide resolved
src/shared/components/query/QueryAndDownloadTabs.tsx Outdated Show resolved Hide resolved
@pvannierop pvannierop force-pushed the hide_download_options branch 2 times, most recently from a38721f to 7ebc85b Compare May 27, 2021 09:16
@alisman
Copy link
Collaborator

alisman commented May 28, 2021

@pvannierop there was some talk about whether we really wanted to disable download?

@pvannierop pvannierop force-pushed the hide_download_options branch 2 times, most recently from 1e38401 to 7c83adc Compare June 4, 2021 06:40
marginLeft: 10,
}}
/>
{!AppConfig.serverConfig.skin_hide_download_controls && (
Copy link
Contributor

@adamabeshouse adamabeshouse Jun 11, 2021

Choose a reason for hiding this comment

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

showShouldDownloadAndCopyControls()?

{
];

if (!AppConfig.serverConfig.skin_hide_download_controls) {
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldShowDownloadandCopyControls()?

/>
)}
{!this.hasNoSegmentData &&
!AppConfig.serverConfig.skin_hide_download_controls && (
Copy link
Contributor

Choose a reason for hiding this comment

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

same here?

@@ -205,15 +207,18 @@ export class ClinicalDataTab extends React.Component<
<Else>
<ClinicalDataTabTableComponent
initialItemsPerPage={20}
showCopyDownload={true}
showCopyDownload={
Copy link
Contributor

Choose a reason for hiding this comment

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

same here?

@@ -479,6 +480,9 @@ export default class MutationMapper<
: undefined
}
legend={this.legendColorCodes}
showDownloadControls={
!AppConfig.serverConfig.skin_hide_download_controls
Copy link
Contributor

Choose a reason for hiding this comment

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

same?

Copy link
Contributor

@adamabeshouse adamabeshouse left a comment

Choose a reason for hiding this comment

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

see comments

@pvannierop pvannierop force-pushed the hide_download_options branch from f05c1c9 to 1e543af Compare June 15, 2021 12:19
@pvannierop pvannierop force-pushed the hide_download_options branch from 1e543af to 668b88e Compare June 22, 2021 13:42
@pvannierop pvannierop force-pushed the hide_download_options branch 2 times, most recently from db94e1e to b2fea3e Compare July 15, 2021 12:22
@alisman
Copy link
Collaborator

alisman commented Aug 12, 2021

@pvannierop gonna close this now.

@alisman
Copy link
Collaborator

alisman commented Aug 12, 2021

@pvannierop i closed this for housekeeping.

@alisman alisman closed this Aug 12, 2021
@pvannierop pvannierop reopened this Dec 10, 2021
@pvannierop pvannierop force-pushed the hide_download_options branch 4 times, most recently from 3777c82 to a57df61 Compare May 18, 2022 14:19
@alisman
Copy link
Collaborator

alisman commented May 18, 2022

@pvannierop is there a reason we don't just do this in one place inside of the component? THere is something called the React context api which would allow us access global config. It strikes me as a perfect usecase for this.

@pvannierop
Copy link
Contributor Author

@alisman I am not aware of this feature. Should we discuss how to do this?

@pvannierop pvannierop closed this May 18, 2022
@pvannierop pvannierop reopened this May 18, 2022
@pvannierop pvannierop force-pushed the hide_download_options branch 5 times, most recently from bf99e71 to f90d864 Compare May 20, 2022 06:42
@@ -1,2 +1,2 @@
#export CBIOPORTAL_URL="http://www.cbioportal.org"
export CBIOPORTAL_URL="http://localhost:8080"
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably shouldn't commit this?

@pvannierop pvannierop force-pushed the hide_download_options branch from f5d2032 to 935e710 Compare May 20, 2022 15:24
@alisman alisman force-pushed the hide_download_options branch from 18e7074 to 8f1ae84 Compare May 24, 2022 14:00
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.

3 participants