Issue #2349: Portfolio templates - [RJM] #2431

rachidatecs · 2024-07-05T22:13:24Z

Ticket

Resolves #2349

Changes

Moved a bunch of duplicate waffle flag checks to the global context_processors
Move/add path, portfolio id, and is org user to the global context_processors since those will be used on the nav
Add suborg to domains table (org user only) - View and JS changes
refactor templates and CSS

Context for reviewers

Devs: Follow the org_user_status global context and logo_clickable view context to understand how the different contexts are managed.

Designers: The suborg display still follows the original design. We can revise when we get to org group UI implementation or filter refactoring.

The nav on mobile uses out of the box styles and colors for both portfolio and regular users.

All: You may need to look at the old profile stuff and make sure it still works as expected as I've refactored how we're doing a context that profile uses - @zandercymatics

Setup

Add a portfolio with you as creator, makes sure the waffle flag is turned on.

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Met the acceptance criteria, or will meet them in a subsequent PR
Created/modified automated tests
Added at least 2 developers as PR reviewers (only 1 will need to approve)
Messaged on Slack or in standup to notify the team that a PR is ready for review
Changes to “how we do things” are documented in READMEs and or onboarding guide
If any model was updated to modify/add/delete columns, makemigrations was ran and the associated migrations file has been commited.

Ensured code standards are met (Original Developer)

All new functions and methods are commented using plain language
Did dependency updates in Pipfile also get changed in requirements.txt?
Interactions with external systems are wrapped in try/except
Error handling exists for unusual or missing values

Validated user-facing changes (if applicable)

New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Add at least 1 designer as PR reviewer

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Pulled this branch locally and tested it
Reviewed this code and left comments
Checked that all code is adequately covered by tests
Made it clear which comments need to be addressed before this work is merged
If any model was updated to modify/add/delete columns, makemigrations was ran and the associated migrations file has been commited.

Ensured code standards are met (Code reviewer)

All new functions and methods are commented using plain language
Interactions with external systems are wrapped in try/except
Error handling exists for unusual or missing values
(Rarely needed) Did dependency updates in Pipfile also get changed in requirements.txt?

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewers should not make a duplicate checklist

As a designer reviewer, I have

Verified that the changes match the design intention

Checked that the design translated visually
Checked behavior
Checked different states (empty, one, some, error)
Checked for landmarks, page heading structure, and links
Tried to break the intended flow

Validated user-facing changes as a designer

Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Tested with multiple browsers (check off which ones were used)
- Chrome
- Microsoft Edge
- FireFox
- Safari
(Rarely needed) Tested as both an analyst and applicant user

Screenshots

github-actions · 2024-07-05T22:17:35Z

🥳 Successfully deployed to developer sandbox rjm.

…file context

github-actions · 2024-07-08T23:02:25Z

🥳 Successfully deployed to developer sandbox rjm.

github-actions · 2024-07-09T16:49:54Z

🥳 Successfully deployed to developer sandbox rjm.

CocoByte

Ok - this took a while to read through, but so far the work looks solid.

I do recommend adding comments in our scss overrides, as it is not clear why some of them exist and what they affect. I totally appreciate the organization into separate files (frontend architecture can get spaghetti in a hurry without some housekeeping).

I also really like the new approach with our context for flags. Seems like a good standard overall for waffle flag items.

I plan on doing testing on RJM tomorrow to verify no breakages. But overall, no issues with the code (except a desire for more comments)

zandercymatics

Just a few things. Nice refactor work here

src/registrar/assets/js/get-gov.js

src/registrar/assets/sass/_theme/_base.scss

src/registrar/models/user.py

src/registrar/registrar_middleware.py

src/registrar/templates/home.html

src/registrar/templates/includes/header_selector.html

src/registrar/templates/portfolio_base.html

src/registrar/views/domains_json.py

src/registrar/views/index.py

zandercymatics · 2024-07-16T15:41:50Z

Approved ^^. Just remember to look over some of the comments + implement, particularly this one:
#2431 (comment)

Won't block the review on it, but I'll take a look after you answer!

zandercymatics

Sorry to retract - I found an edge case with the org flag and the feature flag:

CocoByte · 2024-07-16T20:33:44Z

A few things:

BUG? -- I keep getting a 500 error when trying to add a Senior Official to a portfolio. Everything else appears to work in /admin
The header for the Domains table doesn't match the figma (possibly a design update happened after this ticket was made?)

FIGMA:

SANDBOX:

"Domain Groups" and "Members" don't navigate to anything (I don't see these as part of the ACs so I'm thinking this is intentional, but wanted to double-check)

CocoByte

(see comment)

github-actions · 2024-07-16T21:33:14Z

🥳 Successfully deployed to developer sandbox rjm.

rachidatecs · 2024-07-16T21:34:58Z

A few things:

BUG? -- I keep getting a 500 error when trying to add a Senior Official to a portfolio. Everything else appears to work in /admin

The header for the Domains table doesn't match the figma (possibly a design update happened after this ticket was made?)

"Domain Groups" and "Members" don't navigate to anything (I don't see these as part of the ACs so I'm thinking this is intentional, but wanted to double-check)

I think this is a sandbox-speific issue. The code on the sandbox does not have a senior official on portfolios, but a previous PR might have run a migration that allows it
Quite possible, let's see what Gaby says
Intentional

I'll try merging main then pushing, and will check Figma again when I get a chance

Edit: 1 and 2 should be fixed

rachidatecs · 2024-07-16T21:35:17Z

Sorry to retract - I found an edge case with the org flag and the feature flag:

Fixed

github-actions · 2024-07-16T22:07:48Z

🥳 Successfully deployed to developer sandbox rjm.

github-actions · 2024-07-17T00:08:42Z

🥳 Successfully deployed to developer sandbox rjm.

zandercymatics

Nice work but see @CocoByte s suggestions, too

zandercymatics · 2024-07-17T17:40:55Z

src/registrar/assets/sass/_theme/_header.scss

+        > .usa-navbar {
+            // This is a dangerous override to USWDS, necessary because we have a tooltip on the logo
+            overflow: visible;
+        }


(nitpick/optional) Couldn't we this behaviour to a class on the same element that usa-navbar resides in?

zandercymatics · 2024-07-17T17:43:03Z

src/registrar/registrar_middleware.py

+            user_portfolios = Portfolio.objects.filter(creator=request.user)
+            first_portfolio = user_portfolios.first()


Suggested change

user_portfolios = Portfolio.objects.filter(creator=request.user)

first_portfolio = user_portfolios.first()

first_portfolio = Portfolio.objects.filter(creator=request.user).first()

(Optional) you could also combine this field if you want. Optional though

CocoByte

LGTM (just needs linting)

gabo0oo · 2024-07-17T22:00:45Z

Hi Rachid, adding some feedback after a first pass for visual and AC. This is looking so great!

Change background (behind domains table) to #FCFCFC. Note that the domains table background should be white.
Change border above the nav bar (below the usa banner) to A9AEB1 (uswds base-light)
Change focus on Organization in nav bar so that the margin on the left matches the right (right now it’s really close to the end of the org name).
Increase padding between main content and nav bar/header to 32 instead of 24.
Increase margin between Reset button and search component from 16 to 24
Can you match the table data pieces to match the Figma.. I’m seeing width sizes that don’t match the figma for the 1024 viewport size. Note, in case it’s relevant. In a follow on ticket (Add Domain Groups to the Domains Table page in the Portfolio view #2395), we will improve this table to stretch with the viewport for users with large screens. We do want it to switch to the mobile view under 1024.

github-actions · 2024-07-19T16:09:21Z

🥳 Successfully deployed to developer sandbox rjm.

gabo0oo

Looks great!

rachidatecs added 2 commits July 5, 2024 15:28

wip

a1fca00

extended header for org users

619a71f

rachidatecs requested a review from allly-b as a code owner July 5, 2024 22:13

clenup

a7c29f4

rachidatecs requested review from therealslimhsiehdy, Matt-Spence, CocoByte, dave-kennedy-ecs, erinysong and zandercymatics July 5, 2024 22:17

rachidatecs removed request for therealslimhsiehdy, Matt-Spence, CocoByte, dave-kennedy-ecs, erinysong and zandercymatics July 5, 2024 22:18

rachidatecs changed the title ~~Issue #2349: Portfolio templates~~ DRAFT - Issue #2349: Portfolio templates - [RJM] Jul 5, 2024

Fix issues with org_user_status global context and logo_clickable pro…

dcb63d1

…file context

Add suborg aria label and title

59ed58f

rachidatecs changed the title ~~DRAFT - Issue #2349: Portfolio templates - [RJM]~~ Issue #2349: Portfolio templates - [RJM] Jul 9, 2024

rachidatecs requested review from therealslimhsiehdy, Matt-Spence, CocoByte, dave-kennedy-ecs, Katherine-MN, erinysong and zandercymatics July 9, 2024 18:13

CocoByte reviewed Jul 16, 2024

View reviewed changes

zandercymatics reviewed Jul 16, 2024

View reviewed changes

zandercymatics approved these changes Jul 16, 2024

View reviewed changes

zandercymatics requested changes Jul 16, 2024

View reviewed changes

CocoByte assigned zandercymatics Jul 16, 2024

CocoByte requested changes Jul 16, 2024

View reviewed changes

Fix logo tooltip when profile and org user

bdeb006

rachidatecs added 2 commits July 16, 2024 17:56

merge main

af4423c

color change

2ec3fa7

cleanup

58957e1

zandercymatics approved these changes Jul 17, 2024

View reviewed changes

CocoByte approved these changes Jul 17, 2024

View reviewed changes

rachidatecs added 5 commits July 18, 2024 16:24

Hide suborgs on domains for regular users

716f91e

merge main

d8fce10

lint

e3415a3

trigger pr

acf89e7

merge main

c162d81

gabo0oo reviewed Jul 19, 2024

View reviewed changes

gabo0oo approved these changes Jul 19, 2024

View reviewed changes

rachidatecs merged commit 0c8f0b4 into main Jul 22, 2024
10 checks passed

rachidatecs deleted the rjm/2349-portfolio-ui branch July 22, 2024 16:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #2349: Portfolio templates - [RJM] #2431

Issue #2349: Portfolio templates - [RJM] #2431

rachidatecs commented Jul 5, 2024 •

edited by gabo0oo

Loading

github-actions bot commented Jul 5, 2024

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 9, 2024

CocoByte left a comment

zandercymatics left a comment

zandercymatics commented Jul 16, 2024

zandercymatics left a comment •

edited

Loading

CocoByte commented Jul 16, 2024 •

edited

Loading

CocoByte left a comment

github-actions bot commented Jul 16, 2024

rachidatecs commented Jul 16, 2024 •

edited

Loading

rachidatecs commented Jul 16, 2024

github-actions bot commented Jul 16, 2024

github-actions bot commented Jul 17, 2024

zandercymatics left a comment •

edited

Loading

zandercymatics Jul 17, 2024

zandercymatics Jul 17, 2024

CocoByte left a comment •

edited

Loading

gabo0oo commented Jul 17, 2024

github-actions bot commented Jul 19, 2024

gabo0oo left a comment

		user_portfolios = Portfolio.objects.filter(creator=request.user)
		first_portfolio = user_portfolios.first()

	user_portfolios = Portfolio.objects.filter(creator=request.user)
	first_portfolio = user_portfolios.first()
	first_portfolio = Portfolio.objects.filter(creator=request.user).first()

Issue #2349: Portfolio templates - [RJM] #2431

Issue #2349: Portfolio templates - [RJM] #2431

Conversation

rachidatecs commented Jul 5, 2024 • edited by gabo0oo Loading

Ticket

Changes

Context for reviewers

Setup

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Ensured code standards are met (Original Developer)

Validated user-facing changes (if applicable)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Ensured code standards are met (Code reviewer)

Validated user-facing changes as a developer

As a designer reviewer, I have

Verified that the changes match the design intention

Validated user-facing changes as a designer

Screenshots

github-actions bot commented Jul 5, 2024

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 9, 2024

CocoByte left a comment

Choose a reason for hiding this comment

zandercymatics left a comment

Choose a reason for hiding this comment

zandercymatics commented Jul 16, 2024

zandercymatics left a comment • edited Loading

Choose a reason for hiding this comment

CocoByte commented Jul 16, 2024 • edited Loading

CocoByte left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 16, 2024

rachidatecs commented Jul 16, 2024 • edited Loading

rachidatecs commented Jul 16, 2024

github-actions bot commented Jul 16, 2024

github-actions bot commented Jul 17, 2024

zandercymatics left a comment • edited Loading

Choose a reason for hiding this comment

zandercymatics Jul 17, 2024

Choose a reason for hiding this comment

zandercymatics Jul 17, 2024

Choose a reason for hiding this comment

CocoByte left a comment • edited Loading

Choose a reason for hiding this comment

gabo0oo commented Jul 17, 2024

github-actions bot commented Jul 19, 2024

gabo0oo left a comment

Choose a reason for hiding this comment

rachidatecs commented Jul 5, 2024 •

edited by gabo0oo

Loading

zandercymatics left a comment •

edited

Loading

CocoByte commented Jul 16, 2024 •

edited

Loading

rachidatecs commented Jul 16, 2024 •

edited

Loading

zandercymatics left a comment •

edited

Loading

CocoByte left a comment •

edited

Loading