Enh/nxt 4296 Component search #28

xnhp · 2025-12-17T10:41:08Z

No description provided.

Copilot

Pull request overview

This PR refactors port type handling by removing the type field from the internal data model and introducing a dedicated adapter (toNodePreviewPorts) to map kind to type only at component boundaries. The main changes include:

Removed the includeType parameter from toExtendedPortObject function
Created a new nodePreviewAdapter.ts utility to handle the kind → type mapping for NodePreview components
Updated all NodePreview component usages to apply the adapter at the boundary
Removed type field from test expectations and the ExtendedPortType interface

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`org.knime.ui.js/src/util/portDataMapper.ts`	Removed `includeType` parameter and associated logic that added `type` field to port objects
`org.knime.ui.js/src/util/nodePreviewAdapter.ts`	New utility function to adapt ports by adding `type` as alias for `kind`
`org.knime.ui.js/src/util/fuzzyPortTypeSearch.js`	Updated `toExtendedPortObject` calls to remove `includeType` argument
`org.knime.ui.js/src/test/utils/withPorts.js`	Removed `type` field assignment in port mapping
`org.knime.ui.js/src/store/nodeDescription/__tests__/nodeDescription.test.ts`	Removed `type` field from test expectations
`org.knime.ui.js/src/components/workflowMonitor/__tests__/WorkflowMonitorMessage.test.ts`	Added `toNodePreviewPorts` adapter to test expectations
`org.knime.ui.js/src/components/workflowMonitor/WorkflowMonitorMessage.vue`	Applied `toNodePreviewPorts` adapter when passing ports to NodePreview
`org.knime.ui.js/src/components/workflowMetadata/__tests__/WorkflowMetadata.test.ts`	Removed `type` field from test expectations
`org.knime.ui.js/src/components/workflowMetadata/ComponentMetadata.vue`	Applied `toNodePreviewPorts` adapter when constructing node preview data
`org.knime.ui.js/src/components/spaces/SpaceExplorer.vue`	Applied `toNodePreviewPorts` adapter to port props
`org.knime.ui.js/src/components/nodeRepository/NodeTemplate/NodeTemplate.vue`	Applied `toNodePreviewPorts` adapter to port props
`org.knime.ui.js/src/api/gateway-api/generated-api.ts`	Added new API types and methods for component search functionality
`org.knime.ui.js/src/api/custom-types.ts`	Removed `type` field from `ExtendedPortType` interface

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-17T15:34:14Z

org.knime.ui.js/src/api/gateway-api/generated-api.ts



+/**
+ * ...


The JSDoc comment for ComponentSearchItem contains only '...' as a description. Please provide a meaningful description explaining what a ComponentSearchItem represents, such as 'Represents a component in search results with metadata including name, type, ports, and visual information.'

Suggested change

* ...

* Represents a component entry returned in search results, including its name, description, type, ports, and icon information.

Copilot · 2025-12-17T15:34:14Z

org.knime.ui.js/src/api/gateway-api/generated-api.ts

+     */
+    inPorts?: Array<ComponentSearchItemPort>;
+    /**
+     * The node&#39;s output ports.


The comment refers to 'node's output ports' but this is in the ComponentSearchItem interface which represents a component, not a node. The comment should say 'The component's output ports.' for consistency with the inPorts comment on line 1247.

Suggested change

* The node's output ports.

* The component's output ports.

Copilot · 2025-12-17T15:34:14Z

org.knime.ui.js/src/api/gateway-api/generated-api.ts

+     */
+    name: string;
+    /**
+     * The description of the component as given by the component creator.


The comment says 'The description of the component' but this field is in ComponentSearchItemPort and should refer to the port description, not the component description. It should say 'The description of the port as given by the component creator.'

Suggested change

* The description of the component as given by the component creator.

* The description of the port as given by the component creator.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

hriverahdez · 2025-12-18T13:20:40Z

Reasoning behind the PR size:

With these changes I've tried to tackle multiple problems that started surfacing with the component search feature. Most of which can be boiled down to "logic/features that we already have that are very similar but not quite the same". Therefore, I tried to improve composition for reusability. Some examples:

Split apart components into more reusable versions. Like the InfiniteNodeList, which just takes the infinite loading behavior and adds it to the NodeList which doesn't have it.
Move the data mappers into a more explicit, lib-like API, where we can have dedicated data mappers closely related to the feature/use-case they support
Generalize the behavior of the interaction to add a node to the workflow. Currently, there are many ways to do this depending on the user intent and origin of the interaction (e.g a dbl click, or a drag, or if another node is selected, etc). This is also needed because the AddNode command (and the store action that wraps it) supports multiple capabilities, incl adding a component to the canvas

All of these things required moving files around a fair bit. Hope this helps with review

NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

Copilot

Pull request overview

Copilot reviewed 48 out of 48 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

org.knime.ui.js/src/util/dataMappers/componentSearch.ts

org.knime.ui.js/src/components/common/NodeTemplate/NodeTemplate.vue

org.knime.ui.js/src/composables/useAddNodeToWorkflow.ts

org.knime.ui.js/src/store/componentSearch/index.ts

Copilot · 2025-12-18T14:08:44Z

org.knime.ui.js/src/components/nodeRepository/header/NodeRepositoryHeader.vue

        <div class="actions">
+          <ValueSwitch
+            compact
+            :disabled="!nodeRepositoryLoaded"


The ValueSwitch component is disabled when nodeRepositoryLoaded is false, but this may create an accessibility issue. Users may not understand why the control is disabled. Consider adding a tooltip or aria-label to explain the disabled state.

Suggested change

:disabled="!nodeRepositoryLoaded"

:disabled="!nodeRepositoryLoaded"

:title="

!nodeRepositoryLoaded

? 'The node repository is still loading. You can switch the search context once loading has finished.'

: undefined

"

:aria-label="

!nodeRepositoryLoaded

? 'Search context switcher is disabled until the node repository has finished loading.'

: 'Switch search context between nodes and components.'

"

Copilot · 2025-12-18T14:08:44Z

org.knime.ui.js/src/components/common/NodeList/InfiniteNodeList.vue

+};
+
+const props = withDefaults(defineProps<Props>(), {
+  searchScrollPosition: 0,


Default prop 'searchScrollPosition' is defined but this prop doesn't exist in the Props type definition. This appears to be leftover from refactoring and should be removed.

Suggested change

searchScrollPosition: 0,

this is true and very annoying that withDefaults does not complain type-wise. I saw more default by destruction but well some people like their props object (like: const { something = 0 } = defineProps() that seems to be better type checked.

NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

sonarqubecloud · 2025-12-18T15:38:25Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 85%)

See analysis details on SonarQube Cloud

NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

carlos22 · 2025-12-19T06:39:59Z

org.knime.ui.js/src/components/common/InfiniteLoadingList/InfiniteLoadingList.vue

+  searchScrollPosition.value = position;
+};
+
+const scroller = ref<InstanceType<typeof ScrollViewContainer> | null>(null);


why not use useTemplateRef

It comes from older code so this line didn't change. But yeah, makes sense

carlos22 · 2025-12-19T06:40:37Z

org.knime.ui.js/src/components/common/NodeList/InfiniteNodeList.vue

+};
+
+const props = withDefaults(defineProps<Props>(), {
+  searchScrollPosition: 0,


this is true and very annoying that withDefaults does not complain type-wise. I saw more default by destruction but well some people like their props object (like: const { something = 0 } = defineProps() that seems to be better type checked.

carlos22 · 2025-12-19T06:57:46Z

org.knime.ui.js/src/components/nodeRepository/header/NodeRepositoryHeader.vue

+            compact
+            :disabled="!nodeRepositoryLoaded"
+            :model-value="searchContext"
+            :possible-values="[


there will be icons eventually right?

Unfortunately, no. The ValueSwitch does not support that at all. I still need to sync with Peter but the design in Figma I've seen uses the whole word like I'm doing now

carlos22 · 2025-12-19T06:59:06Z

org.knime.ui.js/src/components/nodeRepository/NodeSearchResults/SidebarNodeSearchResults.vue

@@ -56,7 +56,7 @@ const focusFirst = () => {
  searchResults.value?.focusFirst();
 };

-const onHelpKey = (node: NodeTemplateWithExtendedPorts) => {
+const onShowNodeDetails = (node: NodeTemplateWithExtendedPorts) => {


rename was long overdue :D

carlos22 · 2025-12-19T06:59:49Z

org.knime.ui.js/src/components/nodeRepository/NodeRepository.vue


-import CategoryTree from "./CategoryTree.vue";
 import NodeRepositoryLoader from "./NodeRepositoryLoader.vue";
+import NodeRepositoryTree from "./NodeRepositoryTree/NodeRepositoryTree.vue";


also this aligns better with the other names I agree!

carlos22 · 2025-12-19T07:00:22Z

org.knime.ui.js/src/components/nodeRepository/NodeRepository.vue

    <template v-if="nodeRepositoryLoaded">
      <SidebarSearchResults
        v-if="searchIsActive"
+        v-show="searchContext === 'nodes'"


so we keep it in the DOM, why? To have it faster back on switch? Why don't wrap all of them (3 types) and do the show on the wrapper? Or do the if on the template (but that might be slower)? Its a bit complex having if and show at the same time on 3 items conditionally...
And if css is killing you with the wrapper display: contents is very handy 💅

My main motivation is to have faster switch back, yes. Because these components are sort of heavy and they do a lot of loading upon mount (you can feel it pause briefly when you switch). I want to avoid that and given that the nodes will be the default, it's why I added the active prop for the comp search.

Why don't wrap all of them (3 types) and do the show on the wrapper?

I was doing this before, but wrapping on a div breaks the infinite scrolling because the wrapper div would now need to have height:100%, which tbh is not much but it's rather disconnected from the internal behavior.

Or do the if on the template (but that might be slower)?

Not sure what you meant by this.

But yes, I agree that having 3 v-shows is not very nice. I'll think about the wrapper div again even if I have to add a height to it. Maybe try out your fancy display: contents 😆

carlos22 · 2025-12-19T07:03:56Z

org.knime.ui.js/src/components/nodeSearch/SearchResults.vue

  numFilteredOutNodes: number;
  highlightFirst?: boolean;
-  displayMode?: NodeRepositoryDisplayModesType;
+  displayMode?: Exclude<NodeRepositoryDisplayModesType, "tree">;


the other type on the skeleton was different maybe just use the same there?

I don't want to use this type on the skeleton because it's a more generic component and I don't to tie it to the noderepository in any way.

Overall, I think the problem is that these types are a bit awkward to define. Because the settings (store context) are a super set of what the component uses so they related somewhat. But with the current folder structure the component and stores are "too far away" from each other instead of being more self-contained and related.

This is one of the motivations I have for (later) moving to a different structure similar to what I'm proposing for the hub atm

carlos22 · 2025-12-19T07:06:11Z

org.knime.ui.js/src/composables/useAddNodeToWorkflow.ts

+    toastPresets.workflow.addNodeToCanvas({ error });
+  };
+
+  const addNodeByPosition = async (


nice to have it all in the same place - I know you wanted to to that for a longer time now...

- keyboard navigation - error handling - empty state NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

Copilot AI review requested due to automatic review settings December 17, 2025 10:41

Copilot AI reviewed Dec 17, 2025

View reviewed changes

xnhp force-pushed the enh/NXT-4296-call-search-service branch from 12d8e31 to 424a027 Compare December 17, 2025 10:43

Copilot AI review requested due to automatic review settings December 17, 2025 15:33

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings December 17, 2025 17:37

Copilot AI reviewed Dec 17, 2025

View reviewed changes

hriverahdez force-pushed the enh/NXT-4296-call-search-service branch from be3930e to 7f89cab Compare December 17, 2025 17:38

Copilot AI review requested due to automatic review settings December 18, 2025 11:16

Copilot AI reviewed Dec 18, 2025

View reviewed changes

hriverahdez changed the title ~~Enh/nxt 4296 call search service~~ Enh/nxt 4296 Component search Dec 18, 2025

xnhp and others added 7 commits December 18, 2025 14:23

generated code

542baae

required props

86f8bc2

capitalisation of enum values

4958bcd

NXT-4296: improve component composition and reusability

5ea9854

NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

NXT-4296: setup structure for component search

72ddf2c

NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

NXT-4296: rework data mapper utils

cece9e2

NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

NXT-4296: rework add node to canvas logic

cf2d489

NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

hriverahdez force-pushed the enh/NXT-4296-call-search-service branch from 7921794 to cf2d489 Compare December 18, 2025 13:23

use real data in FE

88e14aa

Copilot AI review requested due to automatic review settings December 18, 2025 14:05

Copilot AI reviewed Dec 18, 2025

View reviewed changes

NXT-4296: move ExtendedPortTypes into data mappers

9ba8901

NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

NXT-4296: remove leftovers from mocked api

f2ed3a0

NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

Copilot AI review requested due to automatic review settings December 18, 2025 16:18

Copilot AI reviewed Dec 18, 2025

View reviewed changes

carlos22 reviewed Dec 19, 2025

View reviewed changes

NXT-4296: improvements

b811725

- keyboard navigation - error handling - empty state NXT-4296 (Gateway API for component search (use HubClient SDK to call Search Service; aggregate and map entities for FE; error handling))

	* ...
	* Represents a component entry returned in search results, including its name, description, type, ports, and icon information.

	* The description of the component as given by the component creator.
	* The description of the port as given by the component creator.

-            :disabled="!nodeRepositoryLoaded"
+            :disabled="!nodeRepositoryLoaded"
+            :title="
+              !nodeRepositoryLoaded
+                ? 'The node repository is still loading. You can switch the search context once loading has finished.'
+                : undefined
+            "
+            :aria-label="
+              !nodeRepositoryLoaded
+                ? 'Search context switcher is disabled until the node repository has finished loading.'
+                : 'Switch search context between nodes and components.'
+            "

Enh/nxt 4296 Component search #28

Are you sure you want to change the base?

Enh/nxt 4296 Component search #28

Uh oh!

Conversation

xnhp commented Dec 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

hriverahdez commented Dec 18, 2025

Reasoning behind the PR size:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Dec 18, 2025

Quality Gate failed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hriverahdez Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

hriverahdez Dec 19, 2025 •

edited

Loading