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
Add create eventing channel via add flow #5950
Add create eventing channel via add flow #5950
Conversation
/assign @invincibleJai |
/kind feature |
Hi @karthikjeeyar I think this is looking great so far. We are going to have a final review on the UX side for final recommendations and help text, but I understand if that needs to come in a later PR. |
@karthikjeeyar @rachael-phillips quick question on this? Is it possible for us to use a heading which is the channel type immediately below the type dropdown? Right now it looks like it's only shown when thrown into YAML, but I think you could use is consistently between either mode. I'm not sure if the mocks included that or not? |
Hey @serenamarie125 thanks for pointing that out because I think you're right that there is a consistency problem to be solved here. I looked into this across the console, and what I see we do in other instances of YAML across the console is no 2nd header. So while I thought originally this would help clarify the selection across the flow, now I am seeing we don't have that pattern as a precedent. That being said, would you be comfortable with removing the 2nd header from the YAML view in this design for consistency? |
frontend/packages/knative-plugin/src/components/add/channels/sections/YamlSection.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/knative-plugin/src/components/add/channels/ChannelForm.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/knative-plugin/src/utils/create-channel-utils.ts
Outdated
Show resolved
Hide resolved
frontend/packages/knative-plugin/src/components/add/channels/ChannelForm.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/knative-plugin/src/utils/create-channel-utils.ts
Outdated
Show resolved
Hide resolved
frontend/packages/knative-plugin/src/utils/create-channel-utils.ts
Outdated
Show resolved
Hide resolved
3095ed1
to
8aa9bf5
Compare
8aa9bf5
to
48b4f32
Compare
/retest |
551401e
to
8b61935
Compare
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.
8b61935
to
3e593f5
Compare
@invincibleJai @vikram-raj Moved default channel always to the top |
3e593f5
to
15d0431
Compare
Hey @karthikjeeyar this is a super small thing, but it should say "Create a Knative Channel.." instead of "Create a Channel.." |
71eced1
to
5f0c3a4
Compare
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.
5f0c3a4
to
bde2b5b
Compare
@vikram-raj Fixed it. |
/retest Please review the full test history for this PR and help us cut down flakes. |
11 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
29bad51
to
0dbd6e6
Compare
0dbd6e6
to
60de7bf
Compare
60de7bf
to
f4c92eb
Compare
/retest |
/test e2e-gcp-console |
Verified the changes works as expected. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: invincibleJai, karthikjeeyar, vikram-raj 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:
Approvers can indicate their approval by writing |
Fixes:
https://issues.redhat.com/browse/ODC-4170,
https://issues.redhat.com/browse/ODC-4305 (yaml submission bug)
Analysis / Root cause:
Add creation of the channels via Add flow
Solution:
defaults-ch-webhook
configmap.Screenshots:
Add Page Tile:
Auto selecting the default channel
Channel Type dropdown
Yaml section
adding channel
Setup to run as basic user:
Create a ClusterRole with the following rules:
Unit Test coverage
cc: @rachael-phillips @beaumorley @serenamarie125 @christianvogt @openshift/team-devconsole-ux