Skip to content
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

EnvFrom in the Env Editors Interface Issues #2182

Closed
6 tasks done
cdcabrera opened this issue Sep 28, 2017 · 18 comments
Closed
6 tasks done

EnvFrom in the Env Editors Interface Issues #2182

cdcabrera opened this issue Sep 28, 2017 · 18 comments
Assignees
Labels
area/usability kind/bug Categorizes issue or PR as related to a bug. priority/P2
Milestone

Comments

@cdcabrera
Copy link
Contributor

cdcabrera commented Sep 28, 2017

EnvFrom

  • The drag and drop handle associated with updating the sequence order of EnvFrom values doesn't remain hidden when there is a single entry
  • Missing link for viewing Config Maps and Secret details. It would be helpful to see these details at the time of adding these bulk additions.
  • Minor copy revisit
  • Minor CSS color highlight and contrast issue on disabled selected values. The color needs adjustment, either the copy or the background... or both.
  • Prefixes for EnvFrom values, see issue Need to handle envFrom prefix #2362

EnvFrom & Variables

  • Random refresh issue where a background process appears to run and reset some of the drop-downs causing them to collapse even though the user has them open
    • This refresh goes towards a larger issue around how watches on the parent view are providing updates, which also happens to affect the setting of individual environment values.
  • On saving, dependent on background processes, there appears to be a random flash of a hidden form. This also appears to happen when saving environment variables.
@cdcabrera cdcabrera added the kind/bug Categorizes issue or PR as related to a bug. label Sep 28, 2017
@cdcabrera cdcabrera self-assigned this Sep 28, 2017
@spadgett spadgett added this to the 3.7.0 milestone Sep 28, 2017
@cdcabrera
Copy link
Contributor Author

@sg00dwin

As discussed the initial fix around removing the disabled and active classes works.

However there appears to be a coincidental case when those 2 classes will still appear, still working on it from the JS side.

As a general heads up...

It's a case where a user has:

  • selected the "first entry" in the Config Map and Secrets list,
  • then goes to add a second one
  • the drop down list goes ahead and highlights the first entry which just so happens to be disabled (at least, that's how it's appearing on my local)

Attached a Gif
oct-02-2017 11-30-01

@cdcabrera
Copy link
Contributor Author

@beanh66 @spadgett in process for completing some of the copy updates referenced around #2198 , wanted to make sure understood correctly about the sorting between Config Maps and Secrets.

Right now we're seeing the opposite of what's referenced in the comments regarding the sort around the select lists. Trying to figure out if there are any other factors at play, if you could post a screen capture that would help out.

[ ...noticed the links say "Config Map or Secret" but in the actual dropdown the secrets are listed first so we should be consistent... ]

The closest we could get to seeing something similar was when the select list displays at the bottom of the page:

oct-03-2017 14-56-41

@beanh66
Copy link
Member

beanh66 commented Oct 3, 2017

@cdcabrera Maybe I'm just running an old version! What you have looks great :)

sg00dwin added a commit to sg00dwin/origin-web-console that referenced this issue Oct 9, 2017
…ile.

Some adjustments needed for vertical alignment.
Reference openshift#2182
sg00dwin added a commit to sg00dwin/origin-web-console that referenced this issue Oct 9, 2017
…ile.

Some adjustments needed for vertical alignment.
Reference openshift#2182
cdcabrera pushed a commit to cdcabrera/origin-web-console that referenced this issue Oct 12, 2017
…ile.

Some adjustments needed for vertical alignment.
Reference openshift#2182
openshift-merge-robot added a commit that referenced this issue Oct 19, 2017
Automatic merge from submit-queue.

EnvFrom Config Map and Secret Link

EnvFrom Issue Updates for...

EnvFrom issue fixes for CSS, copy, and UX. Updates parts of #2182
- Config Map and Secret Link

Initial work is aimed at re-opening the discussion around behavior... this PR initially... 
- has the link display until the envFrom form aspect is edited, then it hides. 
- when the link is displayed it navigates directly to the secret or config map view

![oct-02-2017 14-32-04](https://user-images.githubusercontent.com/3761375/31093083-8a0a87ac-a77e-11e7-887d-f1cedd3d773d.gif)

@beanh66
@cdcabrera
Copy link
Contributor Author

@beanh66

Attached below for the reveal secret piece...

oct-20-2017 18-25-18

@spadgett

@beanh66
Copy link
Member

beanh66 commented Oct 23, 2017

@cdcabrera Looks good, just feels like we may want a little more space between Value Details and Reveal Secret. Even just 5-10px would help. For cases where the revealed values are much longer, would those values wrap and the modal scroll as needed?

@jeff-phillips-18
Copy link
Member

jeff-phillips-18 commented Oct 23, 2017

@beanh66 Should this look more like the config and bind parameters?

image

@jeff-phillips-18
Copy link
Member

Really weird to me to have the hide/reveal action link in the title area. Shouldn't the title include the name of the secret rather than "Value Details" ?

@beanh66
Copy link
Member

beanh66 commented Oct 23, 2017

@jeff-phillips-18 Good point since it's a modal, that is kinda odd to have actions in the title bar. I don't think we need the secret name in the title bar but it actually would be more consistent to have the Reveal Secret link next to the secret name. @cdcabrera

@cdcabrera
Copy link
Contributor Author

@beanh66 can move the Reveal Secret link no problem.

@beanh66
Copy link
Member

beanh66 commented Oct 23, 2017

@cdcabrera The link a user clicked to get this dialog would say "View Details," correct? Shouldn't the reveal link say "Reveal Values"? @spadgett any thoughts?

@jeff-phillips-18 makes a good point about the "Value Details" not meaning much. Would it be possible to have the modal title be "Secret Details" if a secret and "Config Map Details" if a config map?

@beanh66
Copy link
Member

beanh66 commented Oct 23, 2017

screen shot 2017-10-23 at 10 00 32 am

@cdcabrera
Copy link
Contributor Author

@beanh66 funny you should mention the title changing @spadgett and myself talked about doing that, it is easily done.

As for the navigation to get the modal to pop up... you are correct, it's the "View Details". Changing the reveal to something like "Reveal [Values]" does sound better to me.

@cdcabrera
Copy link
Contributor Author

@beanh66 updated the code... can also apply a singular/plural context to "Reveal Value"/"Reveal Values" when only single entries happen.

oct-23-2017 10-31-08

openshift-merge-robot added a commit that referenced this issue Oct 24, 2017
Automatic merge from submit-queue.

Bug 1505782 - Environment From Fix Drag & Order Display

Fix for making sure the drag and drop/order handles are hidden within a read only display. Related to #2182 

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1505782
@cdcabrera cdcabrera mentioned this issue Oct 25, 2017
2 tasks
openshift-merge-robot added a commit that referenced this issue Oct 30, 2017
Automatic merge from submit-queue.

Allow EnvFrom Prefix

Opened envFrom to allow prefix input.

- [x] Minor display logic updates
- [x] Design/UX modifications

Related to #2182 and closes #2362 

![screen shot 2017-10-25 at 5 06 37 pm](https://user-images.githubusercontent.com/3761375/32026101-d15b6e5c-b9b0-11e7-8405-509d2931056e.png)
openshift-merge-robot added a commit that referenced this issue Oct 30, 2017
Automatic merge from submit-queue.

EnvFrom reveal/hide secrets within display modal

Reveal and hide secret value display within the EnvFrom modal.

Related to issue #2182 

![31894776-594d38c0-b7dd-11e7-89b2-ae614d5a5d6e](https://user-images.githubusercontent.com/3761375/32013910-8ee61cbe-b98a-11e7-97df-f147f35037ec.gif)
@cdcabrera
Copy link
Contributor Author

For the record... EnvFrom updates related to smaller screens, expanding out @sg00dwin

screen shot 2017-10-30 at 11 12 41 am

screen shot 2017-10-30 at 11 15 00 am

screen shot 2017-10-30 at 11 15 11 am

@beanh66
Copy link
Member

beanh66 commented Oct 30, 2017

@cdcabrera Why are there two ? icons once a user initiates an error? Wouldn't there just be a single field level help for the "Prefix" field?

@sg00dwin
Copy link
Member

@beanh66 I'm doing some follow up changes from @jennyhaines, so I'll add the valid prefix message to the 'Prefix' label help tooltip and remove the second one so it will look like.

screen shot 2017-10-30 at 3 19 59 pm

@beanh66
Copy link
Member

beanh66 commented Oct 30, 2017

@sg00dwin perfect, thanks!

openshift-merge-robot added a commit that referenced this issue Nov 1, 2017
Automatic merge from submit-queue.

Follow-on updates from jennyhaines

Requested changes
#2362 (comment) and #2182 (comment)


<img width="755" alt="screen shot 2017-10-30 at 3 49 34 pm" src="https://user-images.githubusercontent.com/1874151/32193045-65a72b30-bd8c-11e7-9704-c763ab161392.png">

<img width="326" alt="screen shot 2017-10-30 at 4 08 01 pm" src="https://user-images.githubusercontent.com/1874151/32193095-8c67a470-bd8c-11e7-832b-6dc4914311ce.png">
openshift-merge-robot added a commit that referenced this issue Nov 1, 2017
Automatic merge from submit-queue.

Add track by to container repeat for env vars

Avoids some issues where the page flickers on updates and prevents the
dropdown from closing when open during a background update.

Fixes one of the flicker problems mentioned in #2182

/kind bug
/assign @jwforres 
cc @cdcabrera
@spadgett
Copy link
Member

spadgett commented Nov 1, 2017

The significant refresh issues have been fixed, and @jennyhaines' style recommendations have been implemented. Closing this issue since all of the concerns have been addressed

@spadgett spadgett closed this as completed Nov 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/usability kind/bug Categorizes issue or PR as related to a bug. priority/P2
Projects
None yet
Development

No branches or pull requests

5 participants