Skip to content

feat: rework linking existing data connector#4084

Merged
lorenzo-cavazzi merged 21 commits into
mainfrom
lorenzo/data-connector-modal
Apr 21, 2026
Merged

feat: rework linking existing data connector#4084
lorenzo-cavazzi merged 21 commits into
mainfrom
lorenzo/data-connector-modal

Conversation

@lorenzo-cavazzi
Copy link
Copy Markdown
Member

@lorenzo-cavazzi lorenzo-cavazzi commented Mar 17, 2026

This PR adds a new page to the Data Connector creation modal to simplify the reuse of existing data connectors. It adds minimal search capabilities and replaces the previous "doi" and "identifier" sections.

image

Reference pitch: https://www.notion.so/renku/Show-link-able-DCs-without-search-1260df2efafc80db95c5d605cc6a02bc

/deploy renku=lorenzo/data-connector-modal

@RenkuBot
Copy link
Copy Markdown
Contributor

You can access the deployment of this PR at https://renku-ci-ui-4084.dev.renku.ch

@lorenzo-cavazzi lorenzo-cavazzi force-pushed the lorenzo/data-connector-modal branch 2 times, most recently from e02ecb5 to f8f7e8b Compare March 26, 2026 15:02
@lorenzo-cavazzi lorenzo-cavazzi force-pushed the lorenzo/data-connector-modal branch from 2cf874d to 76c5e45 Compare April 1, 2026 15:10
@lorenzo-cavazzi lorenzo-cavazzi marked this pull request as ready for review April 1, 2026 15:10
@lorenzo-cavazzi lorenzo-cavazzi requested a review from a team as a code owner April 1, 2026 15:10
@lorenzo-cavazzi lorenzo-cavazzi force-pushed the lorenzo/data-connector-modal branch from 7ae1563 to 8ee5b9a Compare April 14, 2026 09:43
Copy link
Copy Markdown
Contributor

@andre-code andre-code left a comment

Choose a reason for hiding this comment

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

Great work!✨ One small UX thought: the row disappearing silently after linking might feel like a glitch to some users. A shrinking progress bar or a fade animation on the "Linked" badge would help signal that the row is about to go away. It makes the transition feel intentional instead of broken. Not a blocker, just a nice polish opportunity!

}

// Tries to catch all the valid doi cases -- it returns the string directly, but we could consider reusing parseDoi to extract the initial string
export function normalizeAsDoi(input: string): string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be great to add some unit tests for this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point! Added here 51ef6d9

const DC_SEARCH_SLUG_PREFIX = "slug:";
const DC_SEARCH_NAMESPACE_PREFIX = "namespace:";
const DC_SEARCH_DOI_PREFIX = "doi:";
const SUCCESS_MESSAGE_TIMEOUT_MS = 10_000;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggest reducing this from 10_000 → 4_000.
10s feels like a hang, while 4s gives users enough time to register the “Linked” state before the row disappears without causing confusion.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I agree that's a little long. I reduced it here 70d8202 and took the occasion to move constants to a separate file since there were many.

@lorenzo-cavazzi
Copy link
Copy Markdown
Member Author

@andre-code we don't use animations in many places but I agree it would be useful in this case. I did something simple reusing the fade and show Bootstrap classes. 4f18ffb

@andre-code
Copy link
Copy Markdown
Contributor

@andre-code we don't use animations in many places but I agree it would be useful in this case. I did something simple reusing the fade and show Bootstrap classes. 4f18ffb

Beautiful ✨ It helps a lot!!!

Copy link
Copy Markdown
Contributor

@andre-code andre-code left a comment

Choose a reason for hiding this comment

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

LGTM

@lorenzo-cavazzi lorenzo-cavazzi merged commit 2c9fbef into main Apr 21, 2026
35 of 36 checks passed
@lorenzo-cavazzi lorenzo-cavazzi deleted the lorenzo/data-connector-modal branch April 21, 2026 08:43
@RenkuBot
Copy link
Copy Markdown
Contributor

Tearing down the temporary RenkuLab deplyoment for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants