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

Use TablePaginationControls for patient cohort nav #139

Merged

Conversation

adamabeshouse
Copy link
Contributor

image

Checks

  • Follows 7 rules of great commit messages. For most PRs a single commit should suffice, in some cases multiple topical commits can be useful. During review it is ok to see tiny commits (e.g. Fix reviewer comments), but right before the code gets merged to master or rc branch, any such commits should be squashed since they are useless to the other developers. Definitely avoid merge commits, use rebase instead.
  • Follows the Airbnb React/JSX Style guide.
  • Make sure your commit messages end with a Signed-off-by string (this line
    can be automatically added by git if you run the git-commit command with
    the -s option)

@adamabeshouse
Copy link
Contributor Author

@@ -230,6 +234,29 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro

}

private getURL(caseId:string='', studyId:string='', cohort:string[]=[]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe buildUrl would be better name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@@ -247,6 +275,40 @@ export default class PatientViewPage extends React.Component<IPatientViewPagePro
});
}

if (this.patientIdsInCohort && this.patientIdsInCohort.length > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not put this in state?

Copy link
Contributor Author

@adamabeshouse adamabeshouse Jan 27, 2017

Choose a reason for hiding this comment

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

could do that too. I think I have it like this because the other URL parameters (studyId, patientId) are handled like this

nextPageDisabled={indexInCohort === this.patientIdsInCohort.length-1}
lastPageDisabled={indexInCohort === this.patientIdsInCohort.length-1}
onFirstPageClick={() =>
window.location.href = this.getURL(
Copy link
Collaborator

Choose a reason for hiding this comment

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

next week lets try to handle this without page refresh. that's the holy grail and it shouldn't be too hard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah

Signed-off-by: Abeshouse, Adam A./Sloan Kettering Institute <abeshoua@mskcc.org>
@adamabeshouse adamabeshouse merged commit 9db1598 into cBioPortal:integration Jan 27, 2017
@adamabeshouse adamabeshouse deleted the patient-prev-next branch August 7, 2018 17:05
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request May 19, 2022
Small typescript changes, and expose more to import
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request May 20, 2022
Small typescript changes, and expose more to import
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request May 23, 2022
Small typescript changes, and expose more to import
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request May 24, 2022
Small typescript changes, and expose more to import
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request May 25, 2022
Small typescript changes, and expose more to import
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request May 26, 2022
Small typescript changes, and expose more to import
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request May 27, 2022
Small typescript changes, and expose more to import
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request May 27, 2022
Small typescript changes, and expose more to import
alisman pushed a commit to alisman/cbioportal-frontend that referenced this pull request Jun 2, 2022
Small typescript changes, and expose more to import
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request Jun 6, 2022
Small typescript changes, and expose more to import
onursumer pushed a commit to onursumer/cbioportal-frontend that referenced this pull request Jun 6, 2022
Small typescript changes, and expose more to import
alisman pushed a commit to onursumer/cbioportal-frontend that referenced this pull request Jun 8, 2022
Small typescript changes, and expose more to import
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.

2 participants