-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
Va-minimal-footer: add component #937
Changes from 6 commits
210800b
fd1aa87
4cd6665
1b402da
6c495c1
ecb5581
9a3e98f
00d2ea8
3504911
34e36ca
4be0584
611366f
8b81d1f
046bf31
fd86267
81f565f
8c302ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import React from 'react'; | ||
import { getWebComponentDocs, propStructure, StoryDocs } from './wc-helpers'; | ||
|
||
const minimalFooterDocs = getWebComponentDocs('va-minimal-footer'); | ||
|
||
export default { | ||
title: 'Components/Minimal Footer', | ||
id: 'components/va-minimal-footer', | ||
parameters: { | ||
componentSubtitle: `va-minimal-footer web component`, | ||
docs: { | ||
page: () => <StoryDocs data={minimalFooterDocs} />, | ||
}, | ||
}, | ||
}; | ||
|
||
const Template = () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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: |
||
return ( | ||
<va-minimal-footer /> | ||
); | ||
}; | ||
|
||
export const Default = Template.bind(null); | ||
Default.argTypes = propStructure(minimalFooterDocs); |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same also here. There is a PNG and an SVG version in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jamigibbs I removed the .png since it's not being used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @it-harrison There are a couple of other places that a |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import { newE2EPage } from '@stencil/core/testing'; | ||
|
||
describe('va-minimal-footer', () => { | ||
it('renders', async () => { | ||
const page = await newE2EPage(); | ||
await page.setContent('<va-minimal-footer></va-minimal-footer>'); | ||
|
||
const element = await page.find('va-minimal-footer'); | ||
expect(element).toHaveClass('hydrated'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import { newSpecPage } from '@stencil/core/testing'; | ||
import { VAMinimalFooter } from '../va-minimal-footer'; | ||
|
||
describe('va-minimal-footer', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense, thank you! |
||
it('renders', async () => { | ||
const page = await newSpecPage({ | ||
components: [VAMinimalFooter], | ||
html: `<va-minimal-footer />`, | ||
}); | ||
expect(page.root).toEqualHtml(` | ||
<va-minimal-footer> | ||
<mock:shadow-root> | ||
<div class="va-footer"> | ||
<a href="/"> | ||
<img alt="VA.gov home" class="va-logo" src="[object Object]"> | ||
</a> | ||
</div> | ||
</mock:shadow-root> | ||
</va-minimal-footer> | ||
`); | ||
}); | ||
|
||
}); |
danbrady marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
.va-footer { | ||
margin-left: auto; | ||
margin-right: auto; | ||
max-width: 1000px; | ||
background-color: var(--color-primary-darkest); | ||
padding: 10px 8px; | ||
|
||
img.va-logo { | ||
width: 200px; | ||
} | ||
|
||
@media (min-width: 481px) { | ||
padding: 10px 16px; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import { Component, Host, h } from '@stencil/core'; | ||
import vaSeal from '../../assets/va-logo-white.svg'; | ||
|
||
/** | ||
* @componentName Minimal Footer | ||
* @maturityCategory caution | ||
* @maturityLevel candidate | ||
*/ | ||
|
||
@Component({ | ||
tag: 'va-minimal-footer', | ||
styleUrl: 'va-minimal-footer.scss', | ||
shadow: true, | ||
}) | ||
export class VAMinimalFooter { | ||
|
||
render() { | ||
|
||
return ( | ||
<Host> | ||
<div class="va-footer"> | ||
<a href="/" > | ||
<img class="va-logo" src={vaSeal} alt="VA.gov home"/> | ||
</a> | ||
</div> | ||
</Host> | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,12 +12,12 @@ | |
<style type="text/css" media="screen"> | ||
body { | ||
/* For a little breathing room around the components */ | ||
padding: 4rem; | ||
/* padding: 4rem; */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you able to revert this change too? |
||
} | ||
</style> | ||
|
||
</head> | ||
<body> | ||
<my-component first="Stencil" last="'Don't call me a framework' JS"></my-component> | ||
</body> | ||
<va-minimal-footer /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand why this file was changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ug, accidentally committed! |
||
</body> | ||
</html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamigibbs I removed the .png since it's not being used