Va-minimal-footer: add component #937

it-harrison · 2023-11-06T16:17:06Z

Chromatic

https://1777-minimal-footer--60f9b557105290003b387cd5.chromatic.com

Description

Closes 1777

Testing done

local testing in Chrome, Edge, Firefox, Safari

Screenshots

Desktop:

Mobile

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)

…ns-affairs/component-library into 1777-minimal-footer

micahchiang · 2023-11-06T17:43:42Z

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

+      <Host>
+        <div class="va-footer">
+          <a href="/" >
+            <img class="va-logo" src={vaSeal} aria-hidden="true"/>


I think because this is a functional image, and serves as the only content of the anchor tag, instead of aria-hidden="true" it needs some alt text to indicate that it's a link that takes the user to the va.gov home page. Something like alt="VA.gov home"

More info here - https://www.w3.org/WAI/tutorials/images/functional/#example-1-image-used-alone-as-a-linked-logo.

And actually, I think the minimal header needs to be fixed as well -

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

Line 30 in f26ebe3

<img class="va-logo" src={vaSeal} aria-hidden="true"/>

@micahchiang thanks for the heads up. Also, here is a PR to update minimal header.

I left a comment on the other PR and I just wanted to mention here too that we can probably use the existing alt text from the current VA.gov footer logo: "VA logo and Seal, U.S. Department of Veterans Affairs"

There is also a title on the link "Go to VA.gov":

@jamigibbs fixed!

jamigibbs · 2023-11-07T16:35:35Z

packages/web-components/src/index.html

      }
    </style>

  </head>
  <body>
-    <my-component first="Stencil" last="'Don't call me a framework' JS"></my-component>
-  </body>
+<va-minimal-footer />  


I'm not sure I understand why this file was changed?

ug, accidentally committed!

jamigibbs · 2023-11-07T16:38:55Z

packages/storybook/public/assets/va-logo-white.png

Did we have to manually add this image here to the Storybook assets? Or was it moved by the build script? I noticed that there is a PNG and an SVG version in web-components/src/assets but here it's just the PNG.

@jamigibbs I removed the .png since it's not being used

jamigibbs · 2023-11-07T16:39:14Z

packages/web-components/assets/va-logo-white.png

Same also here. There is a PNG and an SVG version in web-components/src/assets but here it's just the PNG.

@jamigibbs I removed the .png since it's not being used

@it-harrison There are a couple of other places that a .png file exists still. Do we want these removed as well?

jamigibbs · 2023-11-07T16:41:58Z

packages/web-components/src/components/va-minimal-footer/test/va-minimal-footer.spec.tsx

+import { newSpecPage } from '@stencil/core/testing';
+import { VAMinimalFooter } from '../va-minimal-footer';
+
+describe('va-minimal-footer', () => {


Do we need a unit test at all for this component? I think we can accomplish the same thing that you're doing here with an e2e test like this:

https://github.com/department-of-veterans-affairs/component-library/blob/main/packages/web-components/src/components/va-button/test/va-button.e2e.ts#L5-L18

@jamigibbs I included the unit test here because the similar e2e test required putting the entire base 64 encoded svg in the test which made it unwieldy.

That makes sense, thank you!

jamigibbs · 2023-11-07T16:50:00Z

packages/storybook/stories/va-minimal-footer.stories.jsx

+  },
+};
+
+const Template = () => {


The component story isn't displaying in the code example:

I think to fix this you'll need to pass in an empty object to story like this:

https://github.com/department-of-veterans-affairs/component-library/blob/main/packages/storybook/stories/va-need-help.stories.jsx#L18-L20

jamigibbs · 2023-11-07T22:27:09Z

@it-harrison Can you add this new component and the English strings that we will want to get translations for to our spreadsheet here? 🙏

https://docs.google.com/spreadsheets/d/1iIg9pP5GG-QFk7DrsyoKJv9GebtYmDuzKf_heqxWks4/edit#gid=0

jamigibbs · 2023-11-08T15:41:33Z

packages/storybook/stories/va-minimal-footer.stories.jsx

+
+const defaultArgs = {};
+
+const Template = ({ }) => {


You can probably just do 🤷‍♀️:

const defaultArgs = {}; const Template = (defaultArgs) => {

Or remove defaultArgs completely. I think the example I linked to before was done a little strange:

jamigibbs · 2023-11-08T15:42:06Z

packages/web-components/src/index.html

@@ -12,7 +12,7 @@
    <style type="text/css" media="screen">
      body {
          /* For a little breathing room around the components */
-          padding: 4rem;
+          /* padding: 4rem; */


Are you able to revert this change too?

jamigibbs

I think dev looks good to me but let's get design approval too before it's merged. cc @danbrady

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

it-harrison · 2023-11-14T16:10:17Z

@danbrady I have updated the css:
desktop:

I'm not sure if the mobile view is correct - the padding changes from 32px/8px to 10px/16px. I got these numbers from looking at the minimal-header, not sure if they are correct:

danbrady · 2023-11-14T16:45:32Z

Thanks, @it-harrison. Looking at the mockup, we want more height to the footer. So let's set the padding to 32px/8px for both mobile and desktop. When the header gets tweaked to fill the width, ee should ensure we're consistent with the padding there too.

add va-minimal-footer

210800b

it-harrison added the minor For a minor Semantic Version change label Nov 6, 2023

it-harrison requested a review from a team as a code owner November 6, 2023 16:17

Merge branch 'main' into 1777-minimal-footer

fd1aa87

it-harrison mentioned this pull request Nov 6, 2023

Minimal Footer - Web Component - Development department-of-veterans-affairs/vets-design-system-documentation#1777

Closed

14 tasks

it-harrison added 2 commits November 6, 2023 11:26

use svg

4cd6665

Merge branch '1777-minimal-footer' of github.com:department-of-vetera…

1b402da

…ns-affairs/component-library into 1777-minimal-footer

micahchiang reviewed Nov 6, 2023

View reviewed changes

it-harrison added 2 commits November 7, 2023 10:08

add alt attr to img

6c495c1

update tests

ecb5581

jamigibbs reviewed Nov 7, 2023

View reviewed changes

jamigibbs requested a review from danbrady November 7, 2023 20:39

update alt/title attrs and tests

9a3e98f

it-harrison added 2 commits November 7, 2023 16:36

update story

00d2ea8

add empty destructuring

3504911

jamigibbs reviewed Nov 8, 2023

View reviewed changes

it-harrison and others added 4 commits November 8, 2023 09:59

restore index.html

34e36ca

restore index.html

4be0584

remove .pngs not needed

611366f

Merge branch 'main' into 1777-minimal-footer

8b81d1f

jamigibbs approved these changes Nov 8, 2023

View reviewed changes

Merge branch 'main' into 1777-minimal-footer

046bf31

danbrady reviewed Nov 9, 2023

View reviewed changes

packages/web-components/src/components/va-minimal-footer/va-minimal-footer.scss Outdated Show resolved Hide resolved

danbrady mentioned this pull request Nov 10, 2023

Minimal Header web component - Development department-of-veterans-affairs/vets-design-system-documentation#1393

Closed

14 tasks

update styles

fd86267

keep padding consistent across screen sizes

81f565f

danbrady approved these changes Nov 14, 2023

View reviewed changes

Merge branch 'main' into 1777-minimal-footer

8c302ce

it-harrison merged commit 03d3fdd into main Nov 15, 2023
6 checks passed

it-harrison deleted the 1777-minimal-footer branch November 15, 2023 13:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Va-minimal-footer: add component #937

Va-minimal-footer: add component #937

it-harrison commented Nov 6, 2023 •

edited by github-actions bot

Loading

micahchiang Nov 6, 2023

it-harrison Nov 7, 2023

jamigibbs Nov 7, 2023 •

edited

Loading

it-harrison Nov 7, 2023

jamigibbs Nov 7, 2023

it-harrison Nov 7, 2023

jamigibbs Nov 7, 2023 •

edited

Loading

it-harrison Nov 7, 2023

jamigibbs Nov 7, 2023 •

edited

Loading

it-harrison Nov 7, 2023

jamigibbs Nov 8, 2023

jamigibbs Nov 7, 2023

it-harrison Nov 7, 2023

jamigibbs Nov 7, 2023

jamigibbs Nov 7, 2023 •

edited

Loading

jamigibbs commented Nov 7, 2023

jamigibbs Nov 8, 2023

jamigibbs Nov 8, 2023

jamigibbs left a comment

it-harrison commented Nov 14, 2023

danbrady commented Nov 14, 2023

Va-minimal-footer: add component #937

Va-minimal-footer: add component #937

Conversation

it-harrison commented Nov 6, 2023 • edited by github-actions bot 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

jamigibbs Nov 7, 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

jamigibbs Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamigibbs Nov 7, 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

Choose a reason for hiding this comment

jamigibbs Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

jamigibbs commented Nov 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamigibbs left a comment

Choose a reason for hiding this comment

it-harrison commented Nov 14, 2023

danbrady commented Nov 14, 2023

it-harrison commented Nov 6, 2023 •

edited by github-actions bot

Loading

jamigibbs Nov 7, 2023 •

edited

Loading

jamigibbs Nov 7, 2023 •

edited

Loading

jamigibbs Nov 7, 2023 •

edited

Loading

jamigibbs Nov 7, 2023 •

edited

Loading