migtools / crane-ui-plugin

OpenShift Dynamic Plugin for Crane UI
1 stars 11 forks source link

Add support for labelIcon, remove duplicate main landmark #70

Closed seanforyou23 closed 2 years ago

seanforyou23 commented 2 years ago

Just some small tweaks to the first step in the form, I noticed the helper text goes away once you interact with it, and if you're like me and get it wrong the first time - you'll have to go back and touch/change the form field in some way to see the helper text again.. I moved it to a help popover for the label in case its helpful.. let me know what you think or if there's a better way to display it.

Also, I noticed we have duplicate main regions - given this plugin embeds within another page that surely already contains a main region, I think we can just omit it from here.

Screen Shot 2022-04-11 at 2 23 15 PM
mturley commented 2 years ago

@seanforyou23 do you mind adding a couple screenshots here for @vconzola to review? Some day we'll get a preview build mode in here :)

seanforyou23 commented 2 years ago
Screen Shot 2022-04-18 at 4 10 25 PM Screen Shot 2022-04-18 at 4 10 08 PM
djzager commented 2 years ago

/test all

djzager commented 2 years ago

Trying to get the openshift bot to show up.

/test all

jmontleon commented 2 years ago

/help

jmontleon commented 2 years ago

/test all

jmontleon commented 2 years ago

/approve

jmontleon commented 2 years ago

/lgtm

jmontleon commented 2 years ago

/retest

openshift-ci[bot] commented 2 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: djzager, jmontleon, pranavgaikwad, seanforyou23

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: - ~~[OWNERS](https://github.com/konveyor/crane-ui-plugin/blob/main/OWNERS)~~ [djzager,jmontleon,pranavgaikwad] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
mturley commented 2 years ago

@vconzola did we ever get content folks from UXD to look at this one?

vconzola commented 2 years ago

@mturley No, I've been waiting since multiple pipelines aren't in scope for tech preview anymore.

mturley commented 2 years ago

@vconzola oh... I missed that, does that mean we aren't supporting stage for tech preview? I was also about to add the "test" pipeline (cutover without quiesce), are we not going to support that either?

Was this decided when I was on PTO or am I forgetting a meeting?

mturley commented 2 years ago

Nevermind, I see Marco's comment in the requirements doc now.