Add Minimal header component #846

ataker · 2023-09-14T14:20:24Z

Chromatic

https://minimal-header-dst2030--60f9b557105290003b387cd5.chromatic.com

Description

Closes Minimal header - 2030

Testing done

Tested in Storybook in Chrome, Safari, Edge, and FIrefox

Screenshots

Acceptance criteria

[ ]

Definition of done

Documentation has been updated, if applicable
A link has been provided to the originating GitHub issue (or connected to it via ZenHub)

packages/web-components/src/components/va-minimal-header/va-minimal-header.tsx

packages/web-components/src/components/va-minimal-header/va-minimal-header.scss

packages/web-components/src/components/va-minimal-header/va-crisis-line-modal.tsx

jamigibbs · 2023-09-14T21:25:40Z

packages/web-components/src/components/va-minimal-header/va-crisis-line-modal.scss

+$red-60v: #b51d09;
+$red-70v: #8b1303;
+
+$small-desktop-screen: 1008px;


Is this the right place to be adding tokens? Do we have these values somewhere else in a central place we could pull from instead?

Could be a good opportunity to add something like a breakpoints.json file for token generation or something.

This might be irrelevant if we are able to just use the existing crisis modal variation from va-modal: https://design.va.gov/storybook/?path=/docs/components-va-modal--crisis-line-modal

jamigibbs · 2023-09-14T21:28:05Z

It might just be me but is the VA icon kind of blurry?

micahchiang

I don't have access to the sketch file that contains the mocks for this. Do we have mockups that handle long header values that get passed in? Currently on mobile, long strings make the header take up a lot of space:

Again, I don't have access to the sketch file for this, so not sure if this is expected or not.

micahchiang · 2023-09-14T22:27:00Z

packages/web-components/src/components/va-minimal-header/va-crisis-line-modal.scss

+$red-60v: #b51d09;
+$red-70v: #8b1303;
+
+$small-desktop-screen: 1008px;


Could be a good opportunity to add something like a breakpoints.json file for token generation or something.

micahchiang · 2023-09-14T22:42:28Z

packages/web-components/src/components/va-minimal-header/va-crisis-line-modal.tsx

@@ -0,0 +1,112 @@
+import { Component, Host, State, h } from '@stencil/core';


Should this be its own component? Doesn't feel like it belongs purely as a child component of the minimal header 🤔

https://design.va.gov/storybook/?path=/docs/components-va-modal--crisis-line-modal

Yeah saw that. We have this ticket to address some crisis line inconsistencies in storybook. It seems like .va-crisis-line and .va-crisis-line-button renders a different version than what we're using in here.

packages/web-components/src/components/va-minimal-header/va-minimal-header.tsx

packages/web-components/src/components/va-minimal-header/va-minimal-header.scss

packages/web-components/src/components/va-minimal-header/va-crisis-line-modal.scss

ataker · 2023-09-15T19:31:13Z

Updated logo to an SVG I found from va.gov

Line height reduced for the header

Andrew565 · 2023-09-22T17:18:12Z

This PR is ready for re-review, and now also includes fixes for #2016.

jamigibbs · 2023-09-25T19:55:50Z

packages/web-components/src/components/va-crisis-line-modal/va-crisis-line-modal.tsx

@@ -0,0 +1,130 @@
+import { Component, Host, State, h } from '@stencil/core';
+import arrowRightSvg from '../../assets/arrow-right-white.svg';
+import { CONTACTS } from '../../../../react-components/src/components/Telephone/contacts';


That react Telephone import (along with the contacts file) should be deprecated/removed now.

This is how we have been doing it on vets-website. Does that work here too?

import { CONTACTS } from '@department-of-veterans-affairs/component-library/contacts';

Alternately, the direct path should be something like (it's in the packages/web-components/src/ folder)

import { CONTACTS } from '../../contacts.ts';

jamigibbs · 2023-09-25T19:57:26Z

packages/web-components/src/components/va-crisis-line-modal/va-crisis-line-modal.tsx

+  styleUrl: 'va-crisis-line-modal.scss',
+  shadow: true,
+})
+export class VACrisisLineModal {


Can we have some tests written for this component?

Good idea, on it!

jamigibbs · 2023-09-25T19:58:07Z

packages/web-components/src/components/va-crisis-line-modal/va-crisis-line-modal.tsx

+                    />
+                    <p>
+                      Call TTY if you have hearing loss{' '}
+                      <strong class="vads-u-margin-left--0p5">


We probably don't want to rely on any Formation classes. Can those styles be moved into the component itself?

Also a good idea!

jamigibbs · 2023-09-25T19:59:03Z

packages/web-components/src/components/va-crisis-line-modal/va-crisis-line-modal.scss

+$red-60v: #b51d09;
+$red-70v: #8b1303;
+
+$small-desktop-screen: 1008px;


Is this the right place for tokens?

I thought about it and couldn't find a better solution for tokens than what Alex did here. These values just don't exist anywhere else that we can use right now, until the remainder of the USWDS tokens are brought into the variables file.

Right now we have a tokens file in the web-components directory, in the CSS Library, and in Formation. Now we are adding random tokens here in a specific component.

It just feels like there are tokens everywhere and adding more here might make dealing with them later more challenging than it already will be.

Can we consider adding the colors we need to the CSS-Library, regenerate the file, and then updating the tokens file in the web-components directory with the new compiled file?

@Andrew565 If you're not going to address this feedback now, then please create a follow up ticket and link to that issue here.

New issue created

jamigibbs · 2023-09-25T20:02:18Z

Do we happen to have a sketch or design file for the crisis line modal? I'm not sure what we should be checking it against:

This reverts commit 48bc847.

ataker · 2023-10-10T19:38:26Z

Addressed @jamigibbs's feedback

jamigibbs

Thank you @ataker! Let's just make sure that we have a designer approval on this too before it gets merged cc @babsdenney @danbrady @LWWright7

babsdenney · 2023-10-16T17:58:42Z

Image	Storybook	Sketch
	Button background color (text side) - #b51d09 Font style - Arial Height of the button - 34px	Button background color (text side) - #981B1E Font style - There doesn't seem to be a font style being applied On VA.gov there the text style in Source sans pro Height of the button - 32px
	The medium view has an issue with the .gov banner link
	.gov banner height - 30px	.gov banner height - 22px
	The crisis line modal icon is present	The crisis line icon is removed on small screens
This is not an issue after getting guidance from Matt. The link and text can remain separated.	Expand link for the .gov banner is present	The expand link for the .gov banner merges with the text to become a link (see above) NOTE: This diverges from the USWDS .gov banner that has the expand link present in small screens
	Storybook showes a 16px space between the logo, vertical line, and title	The sketch document varies is space from 7px - 10px but would think instead that 8px should be used for horizontal spacing between the logo, vertical line, and title
	Storybook is showing more vertical padding on the blue header 20px padding on top and bottom	Sketch is showing padding of 10px on top and 11px bottom but maybe we can adjust that to 10px on top and bottom instead
	The divider is 22px in Storybook	The divider is 38px in Sketch
	The background color for the blue header is #182c54	The background color for the blue header in Sketch is #112E51

humancompanion-usds · 2023-10-17T15:24:54Z

Storybook showes a 16px space between the logo, vertical line, and title
The sketch document varies is space from 7px - 10px but would think instead that 8px should be used for horizontal spacing between the logo, vertical line, and title

I just checked the Sketch file and it looks like what I intended to me: 8px at mobile, 16px at desktop between the vertical line and the elements near it. That's horizontal and vertical. At desktop I measured using the VA logo - that's 32px padding top and bottom, 16px to the line.

I will bring the minimal header from the reference Sketch file into the Component library.

humancompanion-usds · 2023-10-17T15:28:35Z

The divider height was meant to stretch depending on the height of the title. For example:

micahchiang · 2023-10-17T20:15:29Z

@humancompanion-usds - not sure if I missed it, but can you drop a link to the sketch file for this in here so @Andrew565 can reference it?

humancompanion-usds · 2023-10-18T20:32:01Z

The Sketch file is in the issue that this PR is related to.

micahchiang

Checked everything in storybook, and it looks good to me 👍

There's an issue with the official gov banner at the medium break point in story book, but we have an existing ticket to address that here - department-of-veterans-affairs/vets-design-system-documentation#2097

babsdenney · 2023-10-19T19:59:31Z

@Andrew565 I took a look at the new header. Everything looks great except the crisis line button. The color is a bit off and there's also some hover over states that could be adjusted. The Crisis line button is something that hasn't changed, could you match all the specifications from the production crisis line button. The simple header crisis line button can be identical to the VA.gov current header.

https://www.va.gov/

babsdenney · 2023-10-20T17:52:56Z

@Andrew565 This looks so great! Thank you for making the changes! The only little nit-picky thing I see is the hover-over effect on the icon. It is a tad brighter than the one in the main VA header. Looking at the color they chose, it is not a color in the Design System. So that might be something to figure out in the future.

They are using - #b51d09

Could we change that color to what is in VA.gov header in prod?

humancompanion-usds · 2023-10-20T18:28:31Z

Looking at the color they chose, it is not a color in the Design System.

I see we already made this change but, this is a bug. It is never, ever correct to use a color not in the Design System. Period. "They" here is VA, which is us. It's on us to fix it and not propagate it. I understand we're trying to recreate the current version but in future if the current version is broken or wrong we should address those things. Let's file an issue on this to correct it in future.

babsdenney · 2023-10-20T18:51:59Z

@humancompanion-usds Thanks for calling this out! I definitely agree that should have been a flag to figure out how we could align with colors in the Design System. I made a design ticket for that work.

department-of-veterans-affairs/vets-design-system-documentation#2221

babsdenney

Design approved!

ataker added 3 commits September 13, 2023 16:36

Add minimal header

2bae7dd

Update components.d.ts

b47030d

cleanup

bc6b097

ataker added the minor For a minor Semantic Version change label Sep 14, 2023

ataker added 3 commits September 14, 2023 11:34

Merge branch 'main' into minimal-header-dst2030

715de67

Update storybook, clean up subheader

75a6a19

Add tests

25d2433

ataker marked this pull request as ready for review September 14, 2023 18:19

ataker requested a review from a team as a code owner September 14, 2023 18:19

jamigibbs reviewed Sep 14, 2023

View reviewed changes

jamigibbs requested review from babsdenney and LillyBoot September 14, 2023 21:30

micahchiang reviewed Sep 14, 2023

View reviewed changes

Merge branch 'main' into minimal-header-dst2030

19ef5a0

ataker mentioned this pull request Sep 15, 2023

va-official-gov-banner padding issue department-of-veterans-affairs/vets-design-system-documentation#2097

Closed

6 tasks

Merge branch 'main' into minimal-header-dst2030

fe59eeb

ataker added 2 commits September 15, 2023 15:32

PR feedback updates

3dd9508

update ems to px, restore working import

06d1d4d

micahchiang mentioned this pull request Sep 15, 2023

Both v1 and v3 Crisis line modal do not reflect the current design department-of-veterans-affairs/vets-design-system-documentation#2016

Closed

6 tasks

Making va-crisis-line-modal a standalone component

5ad88d2

Andrew565 requested review from jamigibbs and micahchiang September 22, 2023 17:18

Andrew565 added 3 commits September 22, 2023 15:42

Removing extra assets

fda448a

Fix for non-loading SVG file

21e029c

Merge branch 'main' into minimal-header-dst2030

a8107f4

jamigibbs reviewed Sep 25, 2023

View reviewed changes

ataker added 8 commits October 10, 2023 10:20

Update render e2e test

48bc847

Revert "Update render e2e test"

c6de010

This reverts commit 48bc847.

update render e2e test

c119af9

remove unused assetsDirs

07b4d17

Merge branch 'main' into minimal-header-dst2030

f402c44

version bump

5a7d7da

Merge branch 'main' into minimal-header-dst2030

3ba4613

Merge branch 'main' into minimal-header-dst2030

2076dff

jamigibbs approved these changes Oct 12, 2023

View reviewed changes

jamigibbs removed the request for review from LillyBoot October 12, 2023 17:09

Merge branch 'main' into minimal-header-dst2030

876339c

Andrew565 added 2 commits October 18, 2023 16:37

Making design-related adjustments

255f050

Merge branch 'main' into minimal-header-dst2030

56064c2

micahchiang approved these changes Oct 19, 2023

View reviewed changes

Bringing crisis line modal button closer to prod

61fa24b

Changing icon hover color

f6e2b97

babsdenney approved these changes Oct 20, 2023

View reviewed changes

Merge branch 'main' into minimal-header-dst2030

0e2437b

Andrew565 merged commit 3e5baa9 into main Oct 20, 2023
6 checks passed

Andrew565 deleted the minimal-header-dst2030 branch October 20, 2023 20:03

		@@ -0,0 +1,112 @@
		import { Component, Host, State, h } from '@stencil/core';

Add Minimal header component #846

Add Minimal header component #846

Conversation

ataker commented Sep 14, 2023 • edited by jamigibbs Loading

Chromatic

Description

Testing done

Screenshots

Acceptance criteria

Definition of done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamigibbs commented Sep 14, 2023

micahchiang 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

ataker commented Sep 15, 2023

Andrew565 commented Sep 22, 2023

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

jamigibbs Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamigibbs Sep 25, 2023 • edited Loading

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

jamigibbs commented Sep 25, 2023

ataker commented Oct 10, 2023

jamigibbs left a comment

Choose a reason for hiding this comment

babsdenney commented Oct 16, 2023 • edited Loading

humancompanion-usds commented Oct 17, 2023 • edited Loading

humancompanion-usds commented Oct 17, 2023

micahchiang commented Oct 17, 2023

humancompanion-usds commented Oct 18, 2023

micahchiang left a comment

Choose a reason for hiding this comment

babsdenney commented Oct 19, 2023

babsdenney commented Oct 20, 2023

humancompanion-usds commented Oct 20, 2023

babsdenney commented Oct 20, 2023 • edited Loading

babsdenney left a comment

Choose a reason for hiding this comment

ataker commented Sep 14, 2023 •

edited by jamigibbs

Loading

jamigibbs Sep 25, 2023 •

edited

Loading

jamigibbs Sep 25, 2023 •

edited

Loading

babsdenney commented Oct 16, 2023 •

edited

Loading

humancompanion-usds commented Oct 17, 2023 •

edited

Loading

babsdenney commented Oct 20, 2023 •

edited

Loading