Convert task selection to quick search modal in pipeline builder

[karthikjeeyar]

Fixes:

https://issues.redhat.com/browse/ODC-6133

Analysis / Root cause:

Convert the existing task selection dropdown to use the new quick search modal

Solution Description:

1. Created a custom PipelineQuicksearchModal.
2. Extended the quicksearch component to have some configurations like icon, modalPositionOffset and isLimitedList
3. Added autoscroll when cycling through a big list of search results by pressing down arrow in the keyboard.
4. Added temporary Delete decoarator (This will be changed in subsequent PR's based on the UX design).

Note: This PR contains only the Task and ClusterTask resources, tekton hub will be done in a separate PR.

Screen shots / Gifs for design review:

Pipeline_task_selection.mp4

test coverage report:

Updated task selection method in integration test in pipeline builder page pipelineBuilder-page.ts

Test setup:

Visit Pipeline builder page and click on Add Task node.

Browser conformance:

• Chrome
• Firefox
• Safari
• Edge

cc: @bgliwa01 @christianvogt @andrewballantyne

[karthikjeeyar]

cc: @makambalaji Updated the pipeline e2e tests to use quick search modal instead of select dropdown.

[makambalaji]

cc: @makambalaji Updated the pipeline e2e tests to use quick search modal instead of select dropdown.

Thanks @karthikjeeyar for the information

[divyanshiGupta]

Code looks good. Will try it locally and add lgtm if everything works fine.

[divyanshiGupta]

@karthikjeeyar In your screen-recording I see the modal is in the centre but if you look at above screenshot it appears on top for me. In topology page the modal appears in the centre so for consistency I think we should keep it in centre in pipeline-builder page as well.

Also in topology quick-search modal we show the provided by label like this Red Hat, Inc i.e the first letter is in uppercase. We might want to do the same here.

I also noticed that when there are only five items to show as you can see in the above screenshot there is a weird gap shown after the last item. It's not a big issue but it looks weird. If this can be fixed with minimal effort we can do it in this PR.

[divyanshiGupta]

Screen.Recording.2021-07-30.at.10.28.32.PM.mov

@karthikjeeyar I think we want to disable keyboard action for opening the modal as here we dont know where the user might want to add the task i.e they can add it parallelly or side-by-side and if they use the keyboard action to open the modal and add the task it gets added parallelly automatically which is something they might not want.

[divyanshiGupta]

@karthikjeeyar I have added a few comments above. Apart from these the functionality works as expected.

[karthikjeeyar]

@karthikjeeyar In your screen-recording I see the modal is in the centre but if you look at above screenshot it appears on top for me. In topology page the modal appears in the centre so for consistency I think we should keep it in centre in pipeline-builder page as well.

Initially I have added modalPositionOffset to be at the center of the screen, removed it based on Andrew's comment here.

Also in topology quick-search modal we show the provided by label like this Red Hat, Inc i.e the first letter is in uppercase. We might want to do the same here

In the designs we don't have this Provided by <providername> at all, it is showing because of the default topology quick search implementation, we have a separate story to match the modal content exactly with the final design.

[karthikjeeyar]

@divyanshiGupta Added a prop to disable keyboard open and used it in pipelineQuicksearch modal

[divyanshiGupta]

@karthikjeeyar In your screen-recording I see the modal is in the centre but if you look at above screenshot it appears on top for me. In topology page the modal appears in the centre so for consistency I think we should keep it in centre in pipeline-builder page as well.

Also in topology quick-search modal we show the provided by label like this Red Hat, Inc i.e the first letter is in uppercase. We might want to do the same here.

I also noticed that when there are only five items to show as you can see in the above screenshot there is a weird gap shown after the last item. It's not a big issue but it looks weird. If this can be fixed with minimal effort we can do it in this PR.

@bgliwa01 wdyt about this?

[karthikjeeyar]

I also noticed that when there are only five items to show as you can see in the above screenshot there is a weird gap shown after the last item. It's not a big issue but it looks weird. If this can be fixed with minimal effort we can do it in this PR.

@divyanshiGupta Yes, there isn't a extra space needed here, when we are not showing the viewLinks at the bottom.
Updated the code

[divyanshiGupta]

@karthikjeeyar I am getting i18n warnings for both TopologyQuickSearch and PipelineQuickSearch.

[karthikjeeyar]

@karthikjeeyar I am getting i18n warnings for both TopologyQuickSearch and PipelineQuickSearch.

I am not seeing any i18n errors after building the code and re-bridging it. Frontend tests are also passing, so I request you to try build.sh once.

[divyanshiGupta]

@karthikjeeyar In your screen-recording I see the modal is in the centre but if you look at above screenshot it appears on top for me. In topology page the modal appears in the centre so for consistency I think we should keep it in centre in pipeline-builder page as well.

Initially I have added modalPositionOffset to be at the center of the screen, removed it based on Andrew's comment here.

Also in topology quick-search modal we show the provided by label like this Red Hat, Inc i.e the first letter is in uppercase. We might want to do the same here

In the designs we don't have this Provided by <providername> at all, it is showing because of the default topology quick search implementation, we have a separate story to match the modal content exactly with the final design.

@karthikjeeyar I am adding lgtm to this PR since you mentioned we have another story for aligning with UX and the functionality works as expected. But it will be good if you can mention these points in the story so that we can follow up with UX and decide how do we handle these.

/lgtm

[divyanshiGupta]

/label qe-approved
/label px-approved
/label doc-approved

[openshift-ci]

@divyanshiGupta: The label(s) /label doc-approved cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, downstream-change-needed

In response to this:

/label qe-approved
/label px-approved
/label doc-approved

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[divyanshiGupta]

/label docs-approved

[invincibleJai]

nit: we can avoid ? or if condition?

[invincibleJai]

/lgtm
/approve

verified the changes, works as expected

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: divyanshiGupta, invincibleJai, karthikjeeyar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• frontend/OWNERS [invincibleJai]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[andrewballantyne]

/retest

[openshift-bot]

/retest-required

Please review the full test history for this PR and help us cut down flakes.

[bgliwa01]

The modal should work like the one in topology so that there is no dark background and it is centered on the screen that is not including the left navigation.

[andrewballantyne]

The modal should work like the one in topology so that there is no dark background and it is centered on the screen that is not including the left navigation.

@karthikjeeyar please make the desired changes in your next PR.