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

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

Conversation

daniel-cy-lu
Copy link
Contributor

@daniel-cy-lu daniel-cy-lu commented Nov 30, 2023

Issue: Program Entity table rows with multiple values are missing spaces and comma between these values. These rows are regions, countries, institutions, primarySites and cancerTypes

Solution: format the query with array.join() method. The results are values with space and commas in between.

Was able to test countries, primarySites in dev. Couldn't edit programs to test the other three rows. But since the update works on countries and primarySites, it should work for the rest as well.

Screenshot
Showing space and comma between values in primarySites and countries.
Screenshot 2023-11-30 at 4 20 09 PM

Type of Change

  • Bug
  • Dependency updates
  • Feature
  • Refactoring
  • Release candidate
  • Styling
  • Testing
  • Other (please describe)

Checklist before requesting review

  • Design (select one):
    • Matches design:
      • component sizes, spacing, and styles
      • font size, weight, colour
      • spelling has been double checked
    • No design provided, screenshot of changes included in PR
    • No changes to UI
  • Back-end (select one):
    • Back-end PRs merged, tested on develop, and linked in this PR
    • No back-end changes
  • Manually tested changes
  • Added copyrights to new files
  • Connected ticket to PR

…t strings showing up on summary table with no space resolved
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.

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.

@daniel-cy-lu
Copy link
Contributor Author

@demariadaniel Thank you Dan like always!

@daniel-cy-lu daniel-cy-lu merged commit 881d78b into develop Dec 5, 2023
2 checks passed
@daniel-cy-lu daniel-cy-lu deleted the feat/#2681-program-entity-summary-section-with-multiple-values-missing-delimiter branch December 5, 2023 20:05
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