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.

Sign up for GitHub

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

Jump to bottom

FP/737: Polish UI and Add User Functionality (FP-1182 & FP-1198) #514

Merged

rstijerina merged 6 commits into task/FP-737-no-ui from task/FP-737-add-ui

Dec 8, 2021

Contributor

owaisj commented Sep 28, 2021

Overview:

Polish UI of Allocations Manager UI & Add User Saga

Related Jira tickets:


          Add user functionality and styling

2f785fb

codecov bot commented Sep 28, 2021 •

edited

Loading

Codecov Report

Merging #514 (c209193) into task/FP-737-no-ui (7aa9c87) will not change coverage.
The diff coverage is n/a.

@@                Coverage Diff                 @@
##           task/FP-737-no-ui     #514   +/-   ##
==================================================
  Coverage              64.02%   64.02%           
==================================================
  Files                    200      200           
  Lines                   7142     7142           
  Branches                 773      773           
==================================================
  Hits                    4573     4573           
  Misses                  2366     2366           
  Partials                 203      203

Flag	Coverage Δ
unittests	`64.02% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

rstijerina requested review from rstijerina and nathanfranklin

September 28, 2021 18:32

rstijerina added 2 commits

October 5, 2021 13:57


          Merge branch 'task/FP-737-no-ui' into task/FP-737-add-ui

292ec02


          Merge branch 'task/FP-737-no-ui' into task/FP-737-add-ui

c5c857f

rstijerina requested changes

View reviewed changes

client/src/components/Allocations/AllocationsCells.js Outdated Show resolved Hide resolved

...nents/Allocations/AllocationsModals/AllocationsManageTeamModal/AllocationsManageTeamModal.js Show resolved Hide resolved

client/src/components/_common/UserSearchbar/UserSearchbar.js Outdated Show resolved Hide resolved

client/src/redux/reducers/allocations.reducers.js Outdated Show resolved Hide resolved

client/src/redux/reducers/allocations.reducers.js

+                  case 'MANAGE_USERS_INIT':
+                    return {
+                      ...state,
+                      loadingUsernames: {

Member

rstijerina Oct 13, 2021

Could we rename loadingUsernames to be more descriptive? Something like projectLoadingMap

...ocations/AllocationsModals/AllocationsManageTeamModal/AllocationsManageTeamModal.module.scss

+                  border: 0 !important; /* override `.workbench-content .btn-secondary` */
+                  background-color: #9d85ef !important; /* override `.workbench-content .btn-secondary` */
+                  color: white !important; /* override `.workbench-content .btn-secondary` */
+                }
               }
               .listing-wrapper {
                 height: 50vh;

Member

rstijerina Oct 13, 2021

On line 22, there's a right hand border. What's that for? Can we remove that?

...ocations/AllocationsModals/AllocationsManageTeamModal/AllocationsManageTeamModal.module.scss

Comment on lines +6 to +15

+                .update-button {
+                  display: flex;
+                  width: fit-content;
+                  border-radius: 0;
+                  font: bold 12px Roboto, sans-serif;
+                  padding: 0.5rem 1rem;
+                  border: 0 !important; /* override `.workbench-content .btn-secondary` */
+                  background-color: #9d85ef !important; /* override `.workbench-content .btn-secondary` */
+                  color: white !important; /* override `.workbench-content .btn-secondary` */
+                }

Member

rstijerina Oct 13, 2021

We have common button themes defined elsewhere I believe. Can we use those? Maybe btn btn-primary classes?

...ocations/AllocationsModals/AllocationsManageTeamModal/AllocationsManageTeamModal.module.scss

		@@ -1,7 +1,18 @@
		.root {
		max-width: 700px;

Member

rstijerina Oct 13, 2021

We can use size="lg" on the <Modal> instead

...ocations/AllocationsModals/AllocationsManageTeamModal/AllocationsManageTeamModal.module.scss

+                  font-weight: normal;
+                }
+                tr:nth-child(even) td {
+                  background-color: rgba(112, 112, 112, 0.15);

Member

rstijerina Oct 13, 2021

Can you swap this for the css var used on the allocations view?

...ocations/AllocationsModals/AllocationsManageTeamModal/AllocationsManageTeamModal.module.scss

@@ @@ -15,3 +26,29 @@ @@
                 padding: 1em;
                 overflow: auto;
               }
+              .manage-team-table {

Member

rstijerina Oct 13, 2021

We should add width: 100%; to the table element

nathanfranklin requested changes

View reviewed changes

Member

nathanfranklin left a comment

3 things I noticed:

A) right side of user list in dialog is a vertical line

B) We need to move the Manage Team button into View Team dialog instead of having the Manage Team button next to the View Team Button. This was to work around the issue that we don't know the delegate when getting an allocation listing. We should add a follow on ticket to resolve this with Tas team if we don't have one
C) The user searching is off

Member

nathanfranklin commented Oct 22, 2021 •

edited

Loading

For "C) The user searching is off", what i see is

i) we search for userName, email and lastName but not the first name (try with Nathan)
ii) searching for 'email' returns no results for franklin. Do we need to talk with portal.tacc folks? is this known? Or for all of these queries with portal.tacc an exact match query?

Member

rstijerina commented Oct 22, 2021

For "C) The user searching is off", what i see is

i) we search for userName, email and lastName but not the first name (try with Nathan) ii) searching for 'email' returns no results for franklin. Do we need to talk with portal.tacc folks? is this known? Or for all of these queries with portal.tacc an exact match query?

Is the user with that email already on the allocation? I had the search set to not include active members.

Member

nathanfranklin commented Nov 23, 2021 •

edited

Loading

For "C) The user searching is off", what i see is

i) we search for userName, email and lastName but not the first name (try with Nathan) ii) searching for 'email' returns no results for franklin. Do we need to talk with portal.tacc folks? is this known? Or for all of these queries with portal.tacc an exact match query?

Created a Jira issue for searching TAS for users. Route being used here that uses TUP is no longer available for us to use.

owaisj added 3 commits

November 29, 2021 20:49


          Merge branch 'main' of github.com:TACC/Core-Portal into task/FP-737-a…

74894e1

…dd-ui


          Merge branch 'task/FP-737-no-ui' of github.com:TACC/Core-Portal into …

186dde6

…task/FP-737-add-ui


          Text and variable name changes

c209193

rstijerina merged commit beb2804 into task/FP-737-no-ui

rstijerina deleted the task/FP-737-add-ui branch

December 8, 2021 20:13

rstijerina added a commit that referenced this pull request


          Task/FP-737: Manage Allocations (#480)

4f6ef60

* Scaffolding

* stopgap

* No UI; backend ready

* Task/FP-1168: add backend unit tests for add/delete members of TAS projects (subtask of FP-737)  (#467)

* Remove unneeded change

* Fix url for user delete

* Remove logging statements and correct ApiException

Second argument of ApiException is status (i.e. status_code)

* Remove status code in ApiException as TAS returns 200

Main reason for not using TAS return code is that for errors (like
missing user), TAS still returns a 200.  Also, it isn't useful as
we handle all ApiException as 400 and don't log that status_code that
that was pass to ApiException

* Add tests

* Fix pep errors

* Remove change to response for testing purposes

* Fix use of ApiException

* Fix tests

* Improve logging message for forced allocation retrieval

* Fix logging for add/delete of users to allocation

* task/FP-737, task/FP-1180: Use Search component from Projects (#477)

* use projects search; add common component

* revert local dev change

* Task/fp 1183 complete saga redux for removing user (#474)

* Complete saga/redux for removing user

Spinner is shown while deleting and error is shown when failed

* Hide remove button for PI user

* Add unit tests

Refactor some fixture into a fixture file

* Add remove user failure saga test

* Refactor removing of user from team from reducer to saga

* Update client/src/redux/sagas/allocations.sagas.js

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>

Co-authored-by: rstijerina <r.sal.tijerina@gmail.com>

* task/FP-1190: Unified search for FP-737 (#479)

* unified search

* update placeholder

* remove autocomplete package

* merge main package files

* FP/737: Polish UI and Add User Functionality (FP-1182 & FP-1198) (#514)

* Add user functionality and styling

* Text and variable name changes

Co-authored-by: rstijerina <r.sal.tijerina@gmail.com>

* Task/FP-1181: Basic unit tests for front-end components (#555)

* Fix failing tests

* Basic unit test for Allocations Management Modal

Co-authored-by: Nathan Franklin <nfranklin@tacc.utexas.edu>
Co-authored-by: rstijerina <r.sal.tijerina@gmail.com>

rstijerina added a commit that referenced this pull request


          task/FP-737: Manage Allocations (#568)

52bd376

* Task/FP-737: Manage Allocations (#480)

* Scaffolding

* stopgap

* No UI; backend ready

* Task/FP-1168: add backend unit tests for add/delete members of TAS projects (subtask of FP-737)  (#467)

* Remove unneeded change

* Fix url for user delete

* Remove logging statements and correct ApiException

Second argument of ApiException is status (i.e. status_code)

* Remove status code in ApiException as TAS returns 200

Main reason for not using TAS return code is that for errors (like
missing user), TAS still returns a 200.  Also, it isn't useful as
we handle all ApiException as 400 and don't log that status_code that
that was pass to ApiException

* Add tests

* Fix pep errors

* Remove change to response for testing purposes

* Fix use of ApiException

* Fix tests

* Improve logging message for forced allocation retrieval

* Fix logging for add/delete of users to allocation

* task/FP-737, task/FP-1180: Use Search component from Projects (#477)

* use projects search; add common component

* revert local dev change

* Task/fp 1183 complete saga redux for removing user (#474)

* Complete saga/redux for removing user

Spinner is shown while deleting and error is shown when failed

* Hide remove button for PI user

* Add unit tests

Refactor some fixture into a fixture file

* Add remove user failure saga test

* Refactor removing of user from team from reducer to saga

* Update client/src/redux/sagas/allocations.sagas.js

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>

Co-authored-by: rstijerina <r.sal.tijerina@gmail.com>

* task/FP-1190: Unified search for FP-737 (#479)

* unified search

* update placeholder

* remove autocomplete package

* merge main package files

* FP/737: Polish UI and Add User Functionality (FP-1182 & FP-1198) (#514)

* Add user functionality and styling

* Text and variable name changes

Co-authored-by: rstijerina <r.sal.tijerina@gmail.com>

* Task/FP-1181: Basic unit tests for front-end components (#555)

* Fix failing tests

* Basic unit test for Allocations Management Modal

Co-authored-by: Nathan Franklin <nfranklin@tacc.utexas.edu>
Co-authored-by: rstijerina <r.sal.tijerina@gmail.com>

* Task/FP-1309 user new user search endpoint (#572)

* Add TAS searching endpoint

* Add basic unit tests

* Use new endpoint and make temp changes to searchbar

* Fix use of team members before ready

* Improve tests

* Add message to error response

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>

* Add docstr

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>

* Fix error

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>

* Add comment to explain use of Soap and Zeep

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>

* fix linting

* Updated UserSearchbar and AllocationsManageTeamModal to jsx components

* Task/FP-1269: Enable Manage Team modal for Project Delegates (#585)

* rename js files to jsx

* Modal switch between view and manage for pi & delegates

* fix linting

* Fixed unit tests

* Fixed linting

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>
Co-authored-by: Garrett Edmonds <edmondsgarrett@gmail.com>

* Fixed unit tests

* Fixed AddUser allocations saga bug

* Added error handling for AllocationsManageTeamModal

* 737: Improve UI of user searching (#624)

* Removed searchbar placeholder text and added help text under searchbar in AllocationsManageTeamModal

* Simplified JS for text field and updated CSS to accommodate JS changes

* Fixed userSearchbar Add button bug

* task/FP-1635: 737 - Implement final Allocations UI (#636)

* Update View Team link button to secondary button

* Remove modal component from this file and export only manage team table component

* Add manage team modal render here, incorporate tab functionality, and update role column to now be a dropdown

* CSS styling for tabs in View modal

* Change 'Manage Allocations' button to read 'Request New Allocation'

* Remove errant reference to ManageTeamModal

* Fix ManageTeamTable import, remove unused prop, and conditionally render 'Manage Team' tab

* Fixed layout unit test, and moved ManageTeamModal test to AllocationsModals tests

* Changed AllocationsManageTeamModal to AllocationsManageTeamTable and adjusted all references to this component

* Fixed linting

* Fixed adding user bug, removed horizontal scroll on manage team table, replaced dropdown in manage team role column with role text, and removed errant log

* feat(fp-737): global mui tabs, simpler mui jsx (#646)

* feat(fp-1635): remove unused css (#650)

* 737: Use `_common` `<Button>` component in `<UserSearchBar>` component (#647)

* Use common button component in UserSearchBar

* fix linting

* Removed manage team table horizontal scrollability + right-side border, and fixed saga state reference.

* Fix linting

* Fixed state loadingUsernames references to correctly update when adding users to allocation

* Fixed key error for manage team table

* Removed help text at bottom of manage team tab and adjusted modal size to accommodate this

* Fixed another loadingUsernames reference in saga for failing unit test

* 737: (FP-1678) Use "secondary" Button for UserSearchBar (#657)

* fix(fp-1678): secondary button for usersearchbar

* fix(fp-1678): button above (z) input usersearchbar

* 737: Use `_common` `<Button>` component in View Team button  (#649)

* feat(fp-737): use our button for view team button

I am not sure if design knows the color change is coming...

Inquiry: https://tacc-team.slack.com/archives/G01K9ET7JM7/p1653604140397549

* fix(fp-1678): new styles vers. = more button pad.

* fix(fp-1678): use small button

* fix(fp-1678): do not let button overlap `<th>`

* fix(fp-1678): prettier:fix + related comment tweak

* fix(fp-1678): install @tacc/core-styles@0.5.2-c

1. I messed up 0.5.2 publish. After unpublish, I can't republish.
2. I messed up 0.5.2(-)b publish. After unpublish, I can't republish.
3. I published up 0.5.2-c.

I also moved v tag beyond the fix, planning I could republish as 0.5.2.

So GitHub is at 0.5.2. And NPM is at 0.5.2-c.

But there is an 0.6.x from tup-ui/…/core-styles, so 0.5.x ends.

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>

* fix(fp-737): make ui closer to design (#655)

* fix(fp-737): make ui closer to design

* docs(fp-737): note why set button position static

* fix(fp-1678): manage team list wrap height change

This value makes modals height match but only at specific window height.

Setting height of modals is better but led me to change too much code.

Instead, a TACC Modal component could handle height from prop.

* fix(fp-1678): validation error

* fix(fp-1678): add space below manage team table

* fix(fp-1678): revert change to font size

Design, a while back, approved 14px font size for tables.

Reference: #401

* fix(fp-1650): `<td>` vert padding to match design

* fix(fp-1650): port classname validation error fix

Source: 38af56b

* fix(fp-1650):  npm run prettier:fix

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>

* Updated loading state of manage team add button to switch to loading once addUser action is fired

Co-authored-by: Sal Tijerina <r.sal.tijerina@gmail.com>
Co-authored-by: Wesley B <62723358+wesleyboar@users.noreply.github.com>

* Increased allocations table alignment style selector specificity and fixed background styling for disabled 'remove' button in manage team modal

* Frontend error handling for add/remove operations

* Render nulls instead of empty strings in absence of user operation errors

* fix(fp-737): remove bkgd color from disabled link (#659)

* fix(fp-737): remove bkgd color from disabled link

Similar result as d60ff3e, but without overwriting CSS.

* feat(fp-737): prettier:fix

Co-authored-by: Owais Jamil <47395902+owaisj@users.noreply.github.com>
Co-authored-by: Nathan Franklin <nfranklin@tacc.utexas.edu>
Co-authored-by: Nathan Franklin <nathan.franklin@gmail.com>
Co-authored-by: Owais Jamil <ojamil@tacc.utexas.edu>
Co-authored-by: edmondsgarrett <43251554+edmondsgarrett@users.noreply.github.com>
Co-authored-by: Garrett Edmonds <edmondsgarrett@gmail.com>
Co-authored-by: Wesley B <62723358+wesleyboar@users.noreply.github.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet