department-of-veterans-affairs / va-mobile-library

https://department-of-veterans-affairs.github.io/va-mobile-library/
ISC License
0 stars 0 forks source link

[CU] Update to Expo 50 and Storybook 7 #173

Closed narin closed 4 months ago

narin commented 4 months ago

Description of Change

This ticket started out as attempting to fix the Android font loading in #98. I suspected that updating to the latest Expo SDK 50 would resolve this. Doing this resulted in errors in Storybook so I decided to update Storybook to v7 as well.

Testing Packages

Screenshots/Video

Web

https://github.com/department-of-veterans-affairs/va-mobile-library/assets/786704/d2415bd3-4966-4f65-a4aa-f8467d63e685

iOS

https://github.com/department-of-veterans-affairs/va-mobile-library/assets/786704/fcbbe144-aa2a-4954-909a-e7310d2b64ef

Android

https://github.com/department-of-veterans-affairs/va-mobile-library/assets/786704/a7479080-fd93-4452-ad13-6513aa9e1ae4

VA Mobile iOS

VA Mobile Android

Testing

PR Checklist

Code reviewer validation:

Publish

If changes warrant a new version per the versioning guidelines and the PR is approved and ready to merge:

TimRoe commented 4 months ago

Guess we can't quite find out yet, but wonder if the Storybook update might also fix #171 to swing for 6 tickets with one PR.

TimRoe commented 4 months ago

May be good to cut an alpha build and spot check flagship. Nothing in particular sticks out as concerning and the changes are mostly in files that have no bearing, but seems like it'd be good to validate the removal of the App. bit is definitely good--it should be, but was previously a hard red error on load in flagship so probably good to confirm.

TimRoe commented 4 months ago

Don't need to go out of your way, but with the initial round of Link now in this branch, may be worth checking if it fixed #171 to update Storybook/Expo to add another ticket smote by this PR.

narin commented 4 months ago

May be good to cut an alpha build and spot check flagship. Nothing in particular sticks out as concerning and the changes are mostly in files that have no bearing, but seems like it'd be good to validate the removal of the App. bit is definitely good--it should be, but was previously a hard red error on load in flagship so probably good to confirm.

Tested within the App. Moving webStorybookColorScheme() back into utils/index.tsx brought back errors about window being undefined. I moved webStorybookColorScheme() into storybook.tsx and loaded it conditionally within a try/catch which resolved the errors in the App. I also did some cleanup to consolidate the color scheme hook in utils/index.jsx so that we don't have to keep doing webStorybookColorScheme() || useColorScheme() and can instead just import useColorScheme from utils and have the logic handled inside there.

App testing screenshots added in PR description above.

Finally, I also fixed #171. This was due to a typo in the font import for iOS in App.tsx. Should be good for final review.