Edit/Create Projects on page in a popup #1901

jeff-phillips-18 · 2017-08-02T13:13:03Z

Requires openshift/origin-web-common#135

jeff-phillips-18 · 2017-08-02T13:23:42Z

I have not removed the create project and/or edit project pages. Not sure if we want them removed as part of this PR or not.

spadgett · 2017-08-02T13:33:52Z

app/views/projects.html

                              Create Project
-                            </a>
+                            </button>
+                            <origin-modal-popup shown="showNewProjectPanel" modal-title="Create Project" on-close="closeNewProjectPanel()">


As much as possible, I'd like to be consistent in how we handle callbacks for our components. It looks like shown and on-close are handled differently here (as well as on-cancel for create-project).

I could change either of the components but that would need to be done there and not here. Most of the components bind functions using < or =, I used & for the new origin-modal-popup component. Should I change that?

Right, understood. I suggest we should use <. I'd at least like to be consistent with callbacks inside the same component. Here shown is < and onClose is & in the same component.

Sorry, I missed this reviewing openshift/origin-web-common#135

(As an aside, I find callbacks in Angular components generally weird and confusing...)

Wondering if onShown is not better as well for consistent naming.

shown is a flag not a function.

I can rename the controller variable to reduce confusion.

Ah OK please disregard then :)

Fixed and renamed the variables.

spadgett · 2017-08-02T13:35:00Z

app/views/projects.html

@@ -101,7 +105,7 @@ <h2 class="h1">
                                </a>
                              </li>
                              <li role="menuitem">
-                                <a ng-href="project/{{project.metadata.name}}/edit?then=./">
+                                <a ng-href="#" ng-click="editProject(project)">


<a href="" ng-click="editProject(project)">

spadgett · 2017-08-02T13:35:45Z

app/scripts/controllers/projects.js

+    $scope.showEditProjectPanel = false;
+
+    $scope.editProject = function(project) {
+      $scope.edittingProject = project;


editingProject

spadgett · 2017-08-07T20:57:12Z

I have not removed the create project and/or edit project pages. Not sure if we want them removed as part of this PR or not.

If we remove them, we might think about redirects for the old URLs (maybe just /create-project) in case someone has a link / bookmark. Not too worried about it for this PR.

spadgett · 2017-08-07T20:59:18Z

app/views/projects.html

@@ -25,9 +25,13 @@ <h2 class="text-center">Loading...</h2>
                        <h1>My Projects</h1>
                        <div class="projects-options">
                          <div class="projects-add" ng-if="canCreate">
-                            <a href="create-project" class="btn btn-md btn-primary">
+                            <button ng-click="createProject()" class="btn btn-md btn-primary">
+                              <span class="fa fa-plus"></span>


aria-hidden="true"

spadgett · 2017-08-07T20:59:51Z

app/views/projects.html

@@ -101,7 +105,7 @@ <h2 class="h1">
                                </a>
                              </li>
                              <li role="menuitem">
-                                <a ng-href="project/{{project.metadata.name}}/edit?then=./">
+                                <a ng-href="" ng-click="editProject(project)">


nit: href=""

spadgett · 2017-08-07T22:20:32Z

If we remove the create project page, we'll also need to update our e2e tests.

https://github.com/spadgett/origin-web-console/blob/f6611831bc6b9ba10b0b7a7c8b90e66e7ddc940b/test/integration/page-objects/createProject.js

jeff-phillips-18 · 2017-08-08T11:14:39Z

Created issue #1917 to remove the pages.

jeff-phillips-18 · 2017-08-08T11:15:20Z

Removed WIP status now that we have updated the version of origin-web-common.
Ready for final reviews/merge.

spadgett · 2017-08-08T12:24:56Z

Hey @jeff-phillips-18. I pulled the changes down, and everything works well. Thanks.

A couple of small style questions for @openshift/team-ux-review. I won't block the PR on any of these, particularly if they require origin-web-common changes. Just small things I noticed.

Should there be any margin between the + and the text on the button? I'm not sure if there is a standard style for this in Patternfly.

No margin (current):

2px margin:

Should we remove input-lg and btn-lg styles now that the form is only in a dialog?

input-lg (current):

Without input-lg:

I noticed the dialog is spaced farther from the kebab than the actions dropdown and feels a little too far away. I assume the spacing should be about the same.

spadgett · 2017-08-08T12:29:25Z

Added Trello card https://trello.com/c/djoHeMob for QE testing.

spadgett · 2017-08-08T13:08:30Z

[test] to make sure this doesn't break our e2e tests

spadgett · 2017-08-08T13:56:06Z

It looks like we do have padding on the button in the landing page, so we should be consistent here so they look the same.

jeff-phillips-18 · 2017-08-08T14:50:30Z

Fixed the create button spacing.

Added issue for input and button sizes: openshift/origin-web-common#142

spadgett

LGTM except for the typo

spadgett · 2017-08-08T14:55:03Z

app/styles/_buttons.less

@@ -41,3 +41,7 @@
    text-decoration: none;
  }
 }
+
+.icon-button-text {
+  padding-left: 4px;


👍 thanks for making this a generic style

spadgett · 2017-08-08T14:55:21Z

app/views/projects.html

@@ -101,7 +105,7 @@ <h2 class="h1">
                                </a>
                              </li>
                              <li role="menuitem">
-                                <a ng-href="project/{{project.metadata.name}}/edit?then=./">
+                                <a g-href="" ng-click="editProject(project)">


typo: g-href

jeff-phillips-18 · 2017-08-08T15:46:55Z

Fixed popup vertical positioning for projects with descriptions.

openshift-bot · 2017-08-08T15:47:15Z

Evaluated for origin web console test up to 8a5d4fa

spadgett

LGTM

openshift-bot · 2017-08-08T16:10:49Z

Origin Web Console Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_web_console/37/) (Base Commit: 8cb608b) (PR Branch Commit: 8a5d4fa)

spadgett · 2017-08-08T16:15:37Z

[merge]

openshift-bot · 2017-08-08T16:23:13Z

Origin Web Console Merge Results: Waiting: You are in the build queue at position: 1

openshift-bot · 2017-08-08T16:23:14Z

Evaluated for origin web console merge up to 8a5d4fa

jeff-phillips-18 requested review from rhamilto, spadgett and dtaylor113 August 2, 2017 13:13

spadgett reviewed Aug 2, 2017

View reviewed changes

rhamilto approved these changes Aug 2, 2017

View reviewed changes

jeff-phillips-18 force-pushed the project branch 2 times, most recently from c618746 to 976a951 Compare August 4, 2017 15:20

spadgett reviewed Aug 7, 2017

View reviewed changes

jeff-phillips-18 mentioned this pull request Aug 8, 2017

Remove unused create project and edit project pages. #1917

Closed

jeff-phillips-18 force-pushed the project branch from 976a951 to 44b91fc Compare August 8, 2017 11:14

jeff-phillips-18 changed the title ~~WIP: Edit/Create Projects on page in a popup~~ Edit/Create Projects on page in a popup Aug 8, 2017

jeff-phillips-18 force-pushed the project branch from 44b91fc to 7a76660 Compare August 8, 2017 14:43

spadgett reviewed Aug 8, 2017

View reviewed changes

Edit/Create Projects on page in a popup

8a5d4fa

jeff-phillips-18 force-pushed the project branch from 7a76660 to 8a5d4fa Compare August 8, 2017 15:45

spadgett approved these changes Aug 8, 2017

View reviewed changes

openshift-bot merged commit 419c1e6 into openshift:master Aug 8, 2017

Edit/Create Projects on page in a popup #1901

Edit/Create Projects on page in a popup #1901

Uh oh!

Conversation

jeff-phillips-18 commented Aug 2, 2017 • edited by spadgett Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeff-phillips-18 commented Aug 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spadgett Aug 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spadgett commented Aug 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spadgett commented Aug 7, 2017

Uh oh!

jeff-phillips-18 commented Aug 8, 2017

Uh oh!

jeff-phillips-18 commented Aug 8, 2017

Uh oh!

spadgett commented Aug 8, 2017

Uh oh!

spadgett commented Aug 8, 2017

Uh oh!

spadgett commented Aug 8, 2017

Uh oh!

spadgett commented Aug 8, 2017

Uh oh!

jeff-phillips-18 commented Aug 8, 2017

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeff-phillips-18 commented Aug 8, 2017

Uh oh!

openshift-bot commented Aug 8, 2017

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Aug 8, 2017

jeff-phillips-18 commented Aug 2, 2017 •

edited by spadgett

Loading

spadgett Aug 4, 2017 •

edited

Loading

openshift-bot commented Aug 8, 2017 •

edited

Loading