-
Notifications
You must be signed in to change notification settings - Fork 4
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-962] Removes links from the access type column #2713
Conversation
dataset.accessManagement === 'open' ? | ||
'Open Access' : dataset.dac?.dacName, | ||
value: dataset.accessManagement, | ||
id: `${dataset.datasetId}-participant-count`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why the id is named "participant-count" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably a copy-paste from a previous column. We should probably give this a unique name.
data: dataset.accessManagement === 'external' ? | ||
'External to DUOS' : | ||
dataset.accessManagement === 'open' ? | ||
'Open Access' : dataset.dac?.dacName, | ||
value: dataset.accessManagement, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between data and value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data is what is shown to the user, value is the underlying content of the selection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
dataset.accessManagement === 'open' ? | ||
'Open Access' : dataset.dac?.dacName, | ||
value: dataset.accessManagement, | ||
id: `${dataset.datasetId}-participant-count`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably a copy-paste from a previous column. We should probably give this a unique name.
Addresses
Removes links from access type. I decided to move to inlining the ternary because we didn't have a switch utility function - this ideally would be a switch statement.
Summary
Have you read Terra's Contributing Guide lately? If not, do that first.