EnvFrom Config Map and Secret Link #2201

cdcabrera · 2017-10-02T18:28:16Z

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

@beanh66

spadgett · 2017-10-02T21:00:21Z

app/views/directives/edit-environment-from.html

@@ -67,6 +67,11 @@
          role="button"
          aria-label="Delete row"
          ng-click="$ctrl.deleteEntry($index, 1)"></a>
+        <a
+          ng-if="!$ctrl.editEnvironmentFromForm.$dirty"


This won't hide the link if the variables form above is dirty.

This logic should be getting removed since we're headed in the recommended direction of displaying the "details" modal link as soon as a value is selected for the list.... per 20171004 comments

spadgett · 2017-10-02T21:05:52Z

@beanh66 Is it odd that the links just disappear after an edit? I wonder if we shouldn't show the contents in a dialog of somewhere on the current page? Or open the link in a new tab?

beanh66 · 2017-10-04T13:02:10Z

@spadgett When I discussed with @cdcabrera I suggested we have the link appear immediately after the item is selected from the dropdown (before clicking save) so users could click that link to open a modal and view the values prior to committing to saving the changes. Is this no longer a possibility?

spadgett · 2017-10-04T13:05:29Z

@beanh66 I think that's a better approach. The current PR does something different.

@cdcabrera Let's try to work on what @beanh66 suggests

beanh66 · 2017-10-04T14:33:40Z

@spadgett @cdcabrera Was thinking something like this:

cdcabrera · 2017-10-05T14:35:18Z

@sg00dwin

Per discussion looking for a bit of style advice on using the modal setup for header, body, and footer classes... this is what is currently working in the PR where we make use of dialog-title, dialog-body, and modal-footer... do we have a consistent set of classes we use for modals?

Config Map Modal

Secret Modal

cdcabrera · 2017-10-05T14:35:39Z

@spadgett @beanh66 had a question around the Secret display...

~~Should we be displaying the secret values either as hidden or visible (dependent on permissions) when the user hits the appropriate "reveal" link?~~

Edit: Per discussion going to approach this round as just showing the property names, no values

sg00dwin · 2017-10-05T15:18:10Z

@cdcabrera This example uses modal-header
https://rawgit.com/patternfly/patternfly/master-dist/dist/tests/modals.html

cdcabrera · 2017-10-05T20:44:39Z

@spadgett @sg00dwin @beanh66

Modal updates converting Secret display to look similar to Config Maps

Secret

Config Maps

beanh66 · 2017-10-06T12:12:58Z

@spadgett @cdcabrera In general styling looks great! I was originally thinking we would only list the parameters and not show values as well but if we decide to show both like you have here, I think there should be an option to reveal the hidden values, otherwise why have them listed?

spadgett

It might be pretty easy to throw in a "Reveal Secret" link for secrets since you're already decoding the data.

spadgett · 2017-10-13T17:14:57Z

app/views/directives/edit-environment-from.html

+      <h4>{{$ctrl.overlayPaneEntryDetails.metadata.name}}
+        <small class="muted">- {{$ctrl.overlayPaneEntryDetails.kind | humanizeKind : true}}</small></h4>
+
+      <div ng-if="!($ctrl.overlayPaneEntryDetails.data | hashSize)" class="empty-state-message text-center">


s/hashSize/size

@spadgett any contrasting opinion to wrapping the size/lodash filter with hashSize? Or we/I could do one big catch with a massive search and replace... I seem to keep making this slip up

spadgett · 2017-10-13T17:15:30Z

app/views/directives/edit-environment-from.html

+        <small class="muted">- {{$ctrl.overlayPaneEntryDetails.kind | humanizeKind : true}}</small></h4>
+
+      <div ng-if="!($ctrl.overlayPaneEntryDetails.data | hashSize)" class="empty-state-message text-center">
+        The {{$ctrl.overlayPaneEntryDetails.kind | humanizeKind : true}} has no items.


"has no properties."

spadgett · 2017-10-13T17:15:51Z

app/views/directives/edit-environment-from.html

+      <div ng-if="$ctrl.overlayPaneEntryDetails.data | hashSize" class="table-responsive scroll-shadows-horizontal">
+        <table class="table table-bordered table-bordered-columns config-map-table key-value-table">
+          <tbody>
+          <tr ng-repeat="(prop, value) in $ctrl.overlayPaneEntryDetails.data">


nit: indentation

spadgett · 2017-10-13T17:17:53Z

Looks good, just some minor comments.

/test

cdcabrera · 2017-10-16T14:54:55Z

@spadgett per quick discussion we'll aim to do the reveal secret as a separate PR

spadgett · 2017-10-17T12:27:53Z

I was able to create this error viewing details for at a config map with your changes. It seemed to be trying to decode a config map value.

angular.js:14199 Error: Failed to execute 'atob' on 'Window': The string to be decoded is not correctly encoded.
    at secrets.js:82
    at lodash.js:13379
    at lodash.js:4944
    at baseForOwn (lodash.js:3001)
    at Function.mapValues (lodash.js:13378)
    at Object.decodeSecretData (secrets.js:74)
    at EditEnvironmentFrom.ctrl.viewOverlayPanel (editEnvironmentFrom.js:33)
    at fn (eval at compile (angular.js:15126), <anonymous>:4:457)
    at expensiveCheckFn (angular.js:16213)
    at callback (angular.js:26592)

spadgett · 2017-10-17T12:28:50Z

app/scripts/directives/editEnvironmentFrom.js

@@ -27,6 +29,16 @@

    ctrl.setFocusClass = 'edit-environment-from-set-focus-' + uniqueId;

+    ctrl.viewOverlayPanel = function(entry) {
+      ctrl.decodedSecretData = SecretsService.decodeSecretData(entry.data);


We shouldn't need to call this if we're not showing the secret content. For now, I'd just use entry.data and not decode.

spadgett · 2017-10-17T12:41:52Z

app/views/directives/edit-environment-from.html

+    </div>
+    <div class="modal-body">
+      <h4>{{$ctrl.overlayPaneEntryDetails.metadata.name}}
+        <small class="muted">- {{$ctrl.overlayPaneEntryDetails.kind | humanizeKind : true}}</small></h4>


Instead of - use –

spadgett · 2017-10-17T12:42:40Z

app/views/directives/edit-environment-from.html

+        <small class="muted">- {{$ctrl.overlayPaneEntryDetails.kind | humanizeKind : true}}</small></h4>
+
+      <div ng-if="!($ctrl.overlayPaneEntryDetails.data | size)" class="empty-state-message text-center">
+        The {{$ctrl.overlayPaneEntryDetails.kind | humanizeKind : true}} has no properties.


We use lowercase names for kinds in sentences throughout the console, so remove the true flag here.

spadgett · 2017-10-17T12:43:20Z

app/views/directives/edit-environment-from.html

+                  ng-if="$ctrl.overlayPaneEntryDetails.kind === 'ConfigMap'"
+                  content="value"
+                  limit="1024"
+                  newline-limit="20"


Probably want to make these limits smaller since it's shown in a dailog.

heh, agreed!

@sg00dwin

Setup for config map and secret details link. Styling adjustments per @sg00dwin.

cdcabrera · 2017-10-18T13:59:44Z

This PR is sequenced to go in before PR "Environment From Fix Drag & Order Display" #2238

spadgett · 2017-10-19T18:44:52Z

/lgtm

openshift-merge-robot · 2017-10-19T19:33:43Z

Automatic merge from submit-queue.

cdcabrera requested review from spadgett and benjaminapetersen October 2, 2017 18:33

spadgett reviewed Oct 2, 2017

View reviewed changes

cdcabrera force-pushed the issue-envfrom-link branch 2 times, most recently from 6609bca to 23100db Compare October 4, 2017 16:04

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 5, 2017

cdcabrera force-pushed the issue-envfrom-link branch from aee5168 to 65b96ce Compare October 6, 2017 18:42

cdcabrera mentioned this pull request Oct 9, 2017

[WIP] EnvFrom Styling openshift/origin-web-common#216

Closed

cdcabrera force-pushed the issue-envfrom-link branch from fc1c603 to 67fdf59 Compare October 12, 2017 12:12

spadgett reviewed Oct 13, 2017

View reviewed changes

cdcabrera force-pushed the issue-envfrom-link branch from 67fdf59 to 93fa531 Compare October 16, 2017 14:27

cdcabrera changed the title ~~[WIP] EnvFrom Config Map and Secret Link~~ EnvFrom Config Map and Secret Link Oct 16, 2017

cdcabrera force-pushed the issue-envfrom-link branch from 93fa531 to e887e86 Compare October 16, 2017 18:24

spadgett reviewed Oct 17, 2017

View reviewed changes

spadgett requested changes Oct 17, 2017

View reviewed changes

spadgett self-assigned this Oct 17, 2017

EnvFrom Config Map and Secret Link

2e1dbef

Setup for config map and secret details link. Styling adjustments per @sg00dwin.

cdcabrera force-pushed the issue-envfrom-link branch from e887e86 to 2e1dbef Compare October 17, 2017 20:38

spadgett approved these changes Oct 19, 2017

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2017

openshift-merge-robot merged commit 768d51d into openshift:master Oct 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EnvFrom Config Map and Secret Link #2201

EnvFrom Config Map and Secret Link #2201

cdcabrera commented Oct 2, 2017 •

edited

Loading

spadgett Oct 2, 2017 •

edited

Loading

cdcabrera Oct 4, 2017 •

edited

Loading

spadgett commented Oct 2, 2017

beanh66 commented Oct 4, 2017

spadgett commented Oct 4, 2017

beanh66 commented Oct 4, 2017

cdcabrera commented Oct 5, 2017

cdcabrera commented Oct 5, 2017 •

edited

Loading

sg00dwin commented Oct 5, 2017

cdcabrera commented Oct 5, 2017

beanh66 commented Oct 6, 2017

spadgett left a comment

spadgett Oct 13, 2017

cdcabrera Oct 16, 2017

spadgett Oct 13, 2017

spadgett Oct 13, 2017

spadgett commented Oct 13, 2017

cdcabrera commented Oct 16, 2017

spadgett commented Oct 17, 2017

spadgett Oct 17, 2017

spadgett Oct 17, 2017

spadgett Oct 17, 2017

spadgett Oct 17, 2017

cdcabrera Oct 17, 2017

cdcabrera commented Oct 18, 2017

spadgett commented Oct 19, 2017

openshift-merge-robot commented Oct 19, 2017

EnvFrom Config Map and Secret Link #2201

EnvFrom Config Map and Secret Link #2201

Conversation

cdcabrera commented Oct 2, 2017 • edited Loading

spadgett Oct 2, 2017 • edited Loading

Choose a reason for hiding this comment

cdcabrera Oct 4, 2017 • edited Loading

Choose a reason for hiding this comment

spadgett commented Oct 2, 2017

beanh66 commented Oct 4, 2017

spadgett commented Oct 4, 2017

beanh66 commented Oct 4, 2017

cdcabrera commented Oct 5, 2017

cdcabrera commented Oct 5, 2017 • edited Loading

sg00dwin commented Oct 5, 2017

cdcabrera commented Oct 5, 2017

beanh66 commented Oct 6, 2017

spadgett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett commented Oct 13, 2017

cdcabrera commented Oct 16, 2017

spadgett commented Oct 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdcabrera commented Oct 18, 2017

spadgett commented Oct 19, 2017

openshift-merge-robot commented Oct 19, 2017

cdcabrera commented Oct 2, 2017 •

edited

Loading

spadgett Oct 2, 2017 •

edited

Loading

cdcabrera Oct 4, 2017 •

edited

Loading

cdcabrera commented Oct 5, 2017 •

edited

Loading