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

[DT-902] Replace DAC Column with Data Use Column in Data Library Dataset Table #2705

Merged
merged 3 commits into from
Nov 1, 2024

Conversation

cinyecai
Copy link
Contributor

Addresses

https://broadworkbench.atlassian.net/browse/DT-902

Summary

Replaces the DAC column in the Data Library Dataset table with a Data Use column
Column displays Data Use codes (primary and secondary) and has a tooltip explaining each code when you hover over that cell in the table

Screen.Recording.2024-10-31.at.4.19.40.PM.mov

Have you read Terra's Contributing Guide lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@cinyecai cinyecai requested a review from a team as a code owner October 31, 2024 20:21
@cinyecai cinyecai requested review from rushtong and snf2ye and removed request for a team October 31, 2024 20:21
label: `DAC for dataset ${dataset.datasetId}: ${dataset.dac?.dacName}`
})
cellStyle: makeHeaderStyle(cellWidths.dataUse),
cellDataFn: (dataset: DatasetTerm) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This whole function reuses the Data Use logic used on the DAC Datasets table (in the file DACDatasetTableCellData.jsx) to maintain consistency in how each table's Data Use column behaves/looks. It's not as clean as the other cellDataFn's in this file so I'm open to feedback/suggestions on what to do here if this isn't the best approach

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update - Adjusted to using helper function instead

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's okay. I would rather it be different here than duplicate the logic in multiple places.

Copy link
Contributor

@fboulnois fboulnois left a comment

Choose a reason for hiding this comment

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

this looks fantastic, great job 👍

Copy link
Contributor

@s-rubenstein s-rubenstein left a comment

Choose a reason for hiding this comment

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

LGTM

label: `DAC for dataset ${dataset.datasetId}: ${dataset.dac?.dacName}`
})
cellStyle: makeHeaderStyle(cellWidths.dataUse),
cellDataFn: (dataset: DatasetTerm) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's okay. I would rather it be different here than duplicate the logic in multiple places.

@cinyecai cinyecai merged commit e566c65 into develop Nov 1, 2024
9 checks passed
@cinyecai cinyecai deleted the cc-dt-902-replace-dac-column branch November 1, 2024 17:19
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.

4 participants