-
Notifications
You must be signed in to change notification settings - Fork 20
Link Collection & Link Dataset #835
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
base: develop
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR implements link collection and link/unlink dataset functionality, allowing users to create links between collections and datasets. The changes include new modal interfaces, API integration, and visual indicators for linked items.
Key Changes:
- Add link collection functionality with dropdown UI and modal interface
- Add link and unlink dataset functionality with button components and modals
- Add visual indicators (link icons) to collection and dataset cards when items are linked
Reviewed Changes
Copilot reviewed 59 out of 60 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
src/sections/collection/link-collection-dropdown/ |
New dropdown component and modal for linking collections |
src/sections/dataset/dataset-action-buttons/link-and-unlink-actions/ |
New buttons and modals for linking/unlinking datasets |
src/sections/collection/collection-items-panel/items-list/ |
Updated card headers to show link icons |
tests/component/ |
Comprehensive test coverage for new components |
src/stories/ |
Storybook stories for new components |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...onent/sections/dataset/dataset-action-buttons/link-dataset-button/LinkDatasetButton.spec.tsx
Show resolved
Hide resolved
...t/sections/dataset/dataset-action-buttons/unlink-dataset-button/UnlinkDatasetButton.spec.tsx
Outdated
Show resolved
Hide resolved
...onent/sections/dataset/dataset-action-buttons/link-dataset-button/LinkDatasetButton.spec.tsx
Outdated
Show resolved
Hide resolved
tests/component/sections/collection/link-collection-dropdown/LinkCollectionDropdown.spec.tsx
Show resolved
Hide resolved
src/sections/dataset/dataset-action-buttons/link-and-unlink-actions/LinkAndUnlinkActions.tsx
Outdated
Show resolved
Hide resolved
tests/component/sections/collection/link-collection-dropdown/LinkCollectionDropdown.spec.tsx
Outdated
Show resolved
Hide resolved
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.
Great job! I left some comments here for some UI tweaks
) | ||
} | ||
|
||
if (noCollections) { |
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.
if there is no collection to link or unlink, we could hide the save button.
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.
That's already covered since if there is no collection selected the save button remains disabled.
</p> | ||
)} | ||
|
||
{errorLinkingDataset && <small className="text-danger">{errorLinkingDataset}</small>} |
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.
We could us this for errors? we usually used this alert elsewhere
if (errorLinkingDataset) {
return (
<Alert variant="danger" dismissible={false}>
{errorLinkingDataset}
</Alert>
)
}
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.
yeeh but in this case I think I prefer an inline small text showing the error than the error alert and keeping things rendered.
...sections/collection/link-collection-dropdown/collection-link-select/CollectionLinkSelect.tsx
Show resolved
Hide resolved
src/stories/dataset/dataset-action-buttons/link-dataset-button/LinkDatasetButton.stories.tsx
Show resolved
Hide resolved
@ChengShi-1 I've applied some of the changes. Ready for review again, thanks 🙏🏼 |
What this PR does / why we need it:
CollectionCard
andDatasetCard
components to show link icon if linked, we still need to extend the search api to return that boolean.Which issue(s) this PR closes:
Special notes for your reviewer:
The search is debounced by 400ms to avoid searching everytime the input changes.
Suggestions on how to test this:
Step 1: Run the Development Environment
./run-env.sh unstable
otherwise start it with./run-env.sh 11710-find-dataverses-for-linking
.Step 2:
Does this PR introduce a user interface change? If mockups are available, please link/include them here: Yes.
Link Collection
No collections to link

Only One Collection To Link this Collection to

Link & Unlink Dataset
Only One Collection To Link this Dataset to

Only One Collection to unlink this dataset from

Is there a release notes update needed for this change?:
Additional documentation: