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.

Sign up for GitHub

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

bug/#2681-program-entity-summary-section-with-multiple-values-missing-delimiter(comma) #2745

Merged
merged 3 commits into from
Dec 5, 2023
Merged

bug/#2681-program-entity-summary-section-with-multiple-values-missing-delimiter(comma) #2745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
591a230
add space comma in fields that are in array format. Issue with elemen…
daniel-cy-lu Nov 30, 2023
2c49136
remove console
daniel-cy-lu Nov 30, 2023
c07891c
add back husky folder
daniel-cy-lu Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .husky/pre-commit
100755 → 100644
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these deleted? I've had them interfere with my local dev before, but as I remember we still want them in the repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Dan, we are not deleting, but adding space and comma between the row that has multiple values. Right now there is no space between them. See the screenshot in this ticket: https://app.zenhub.com/workspaces/icgc-argo-platform-dk-production-board-5e542d38415f5034e9fed89d/issues/gh/icgc-argo/platform-ui/2681

Copy link
Contributor

Choose a reason for hiding this comment

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

The code for adding commas looks fine, but the PR contains commits which delete our Husky pre-commit / pre-push files

Copy link
Contributor

Choose a reason for hiding this comment

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

2c49136ea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohh right, I am not sure what to do with this. I was playing around because I couldn't push this ticket. let me see if I could fix it. Sorry I forgot to address this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@samrichca last discussed this with you, do you have any suggestions how we can get around this going forward?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@demariadaniel I added back the husky folder and was able to git push this time. I don't the reasoning behind this though.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

npx --no pretty-quick@^2.0.1 --staged
npx --no pretty-quick@^2.0.1 --staged
2 changes: 1 addition & 1 deletion .husky/pre-push
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
. "$(dirname -- "$0")/_/husky.sh"

npm run graphql-codegen
npm run type-check
npm run type-check
10 changes: 5 additions & 5 deletions components/pages/program-entity/ProgramCardsLayout/util.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ export const createProgramSummaryData = (programSummaryQuery: ProgramSummaryQuer
'Program Shortname': programSummaryQuery?.shortName || '',
'Full Program Name': programSummaryQuery?.name || '',
Description: programSummaryQuery?.description || '',
Countries: programSummaryQuery?.countries || '',
'Primary Sites': programSummaryQuery?.primarySites || '',
Countries: programSummaryQuery?.countries?.join(', ') || '',
'Primary Sites': programSummaryQuery?.primarySites?.join(', ') || '',
Website: programSummaryQuery.website ? (
<Link
href={programSummaryQuery.website}
Expand All @@ -35,9 +35,9 @@ export const createProgramSummaryData = (programSummaryQuery: ProgramSummaryQuer
) : (
''
),
Institutions: programSummaryQuery?.institutions || '',
'Processing Regions': programSummaryQuery?.regions || '',
'Cancer Types': programSummaryQuery?.cancerTypes || '',
Institutions: programSummaryQuery?.institutions?.join(', ') || '',
'Processing Regions': programSummaryQuery?.regions?.join(', ') || '',
'Cancer Types': programSummaryQuery?.cancerTypes?.join(', ') || '',
};
};

Expand Down