Initial build of service-list web component #1475
Conversation
packages/web-components/src/components/va-service-list/va-service-list.tsx
Outdated
Show resolved
Hide resolved
packages/web-components/src/components/va-service-list/va-service-list.scss
Outdated
Show resolved
Hide resolved
| min-width: 40px !important; | ||
| max-width: 40px !important; | ||
| color: #ffffff !important; | ||
| background-color: #162e51 !important; |
There was a problem hiding this comment.
question: How do we make the background color dynamic? Looks like the designs have several different background colors shown for different icons.
There was a problem hiding this comment.
I added the icon prop to the class so we can define the background colors for each icon directly in the SCSS file. There might be an alternative approach like dynamically setting the background color inline using a prop such as vads-color-hub-life-insurance, with life-insurance being the passed prop. But I’m keeping it simple for now and we can revisit and adjust the implementation later if needed when integrating this into vets-website
packages/web-components/src/components/va-service-list/va-service-list.tsx
Outdated
Show resolved
Hide resolved
| <a href="/"> | ||
| <div class="header"> | ||
| <slot name="icon"> | ||
| <va-icon class="icon" size={3} icon={icon}></va-icon> |
There was a problem hiding this comment.
since the icon is optional, we will need a way to not render that here if the icon identifier isn't passed in.
| shadow: true, | ||
| }) | ||
| export class VaServiceList { | ||
| @Prop() serviceDetails: any; |
There was a problem hiding this comment.
we should get a type created here for the serviceDetails so that it isn't using any
There was a problem hiding this comment.
Created an interface for ServiceDetails and for ServiceAction as well to avoid using any
| try { | ||
| this.parsedServiceDetails = typeof newValue === 'string' ? JSON.parse(newValue) : newValue; | ||
| } catch (error) { | ||
| console.error('Error parsing serviceDetails:', error); |
There was a problem hiding this comment.
There is a consoleDevError that might be worth using here instead of console.error
| </li> | ||
| )); | ||
|
|
||
| const actionNeeded = serviceStatus === 'Eligible'; |
There was a problem hiding this comment.
I'm not sure about this check here. Would it be more appropriate to instead check if parsedAction has href and text to determine if actionNeeded is truthy?
There was a problem hiding this comment.
yes that would be better. Changed the line to const actionNeeded = parsedAction?.href && parsedAction?.text
| <va-icon class="icon" size={3} icon={icon}></va-icon> | ||
| </slot> | ||
| <slot name="service-name"> | ||
| <h3 class="service-name">{serviceName}</h3> |
There was a problem hiding this comment.
Should this heading level be dynamic? My guess is that an H3 may not work in every scenario, and we should be able to configure this via a serviceNameHeadingLevel prop or similar naming. Creating it with a default level of 3 would also probably be a good default then.
|
Last thing from reviewing this initial work: I think all the slots should be removed. Having named slots means that every slot's content can be replaced by passing in a slot element as a child to the component, which would leave the door open for any markup to be placed in this component. |
Makes sense. I removed all the slots. |
Chromatic
https://create-va-service-list-component--65a6e2ed2314f7b8f98609d8.chromatic.com
Configuring this pull request
major,minor,patch, orignore-for-release).ignore-for-releaseif files haven't been changed in a component library package. (ie. only Storybook files)/packages/coreversion number if this will be the last PR merged before a release.Description
Closes #295
Summary
QA Checklist
Screenshots
Acceptance criteria
Definition of done