Task/86 - Incorporate Icon Component in Flagship App #10400
Conversation
… of VAIcon. Had some complications and will need to do further testing to make sure everything is in working order.
…, PickerList, FullScreenSubtask, ViewMessageScreen and appointments. In updating I noticed that there is no equivalent for the Unread icon in the new Icon library, in the meantime I'm using placeholders to make sure tests don't break. Also, made sure unit tests got fixed and weren't breaking anymore.
…w Icon props as well as related components called by those components. Was able to get local svg to load for the Unread icon. Updated more icons. Still have some clean up and some tests to fix, however.
…StatusScreen and ClickForActionLink. I noticed some of the original color logic was causing some icons to not show on screen because the new Icon doesnt recognize the old ColorVariant colors that well, updating to accept a hex code or one of the vads colors fixed the issue but this required some small updates to props and components.
…ly to keep things neat. Removed unused comments around app.
matt-guest-wilcore
requested review from
timwright12,
alexandec,
theodur,
dumathane,
cadibemma,
Sparowhawk and
DonMcCaugheyUSDS
|
|
|
Yes, some icons will look different. The new Icon library sometimes doesn't have the same icons as the previous so I had to follow the Figma for guidance. Here's an example showing the icons used for links in the app using the new library. Because the VA.gov link opens up an external web page the icon was changed to reflect that. I'm thinking I should get UX to take a look at this just to verify everything is updated as it should. Thanks for bringing this up. |
|
Sounds good! Heres another one I found that was a little weird. |
|
@matt-guest-wilcore @kimmccaskill-oddball thanks for finding those and calling them out! Good catch on removing the icon next to "Only use messages for non-urgent needs" since it's not an external link. The other couple items I found are here in Figma. The checkmark is more important to fix than the slight sizing and alignment issue of the attach icon. Otherwise, nothing else was jumping out at me. |
matt-guest-wilcore
dismissed
kimmccaskill-oddball’s stale review
via
99424ed
Description of Change
Updated every instance of
<VAIcon />in the app to the new<Icon />component from the mobile-component-library dependancy. Doing so also required finding and updating the names of the assets to their new equivalent (ex: RightArrowInCircle is now ArrowCircleRight).I also had to update a few tests after adding the Icon component to some screens. For the components
BaseListItemandVASelectorI had to incorporate the new vads colors in order to get icons to show properly (Some didn't mesh well with the previous theme system).I made an effort to check every screen in the app (in light and dark mode) to make sure the new icons are showing properly. I'd recommend also going through the app yourself in case I missed anything. I'm going to try to get the UX designers to have a look at this and get their input as well.
Screenshots/Video
Testing
Reviewer Validations
PR Checklist
Reviewer: Confirm the items below as you review
For QA
Run a build for this branch