Skip to content
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

Upgrade React Native Paper and Fix Warnings #518

Merged
merged 1 commit into from
Jan 17, 2024
Merged

Conversation

sternetj
Copy link
Contributor

Changes

  • Upgrades React Native Paper to fix the warning about RCTView has a shadow set but cannot calculate shadow efficiently
  • Removes the hidden style from PillarsTile as it also triggered a RCTView has a shadow set but cannot calculate shadow efficiently warning
  • Adds a background and radius to the tracker circles to prevent the RCTView has a shadow set but cannot calculate shadow efficiently warning
  • Adds a background to the message compose modal to avoid the RCTView has a shadow set but cannot calculate shadow efficiently warning
  • Changes the screen params for AdvancedTrackerEditor to be serializable to avoid a Non-serializable values were found in the navigation state. warning
  • Reworks the AdvancedTrackerEditor save button to avoid a React has detected a change in the order of Hooks error when setting the right header option
  • Fixes a NaN error when trendline has zero data points

Screenshots

Here is a video of everything work correctly:

react-native-paperupgrdae-demo.mov

@@ -111,7 +111,6 @@ export const SearchRecipientsModal = ({
mode="contained"
style={styles.doneButton}
labelStyle={styles.doneButtonLabel}
compact={true}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the upgrade this was causing the button text to be truncated

@coveralls
Copy link

coveralls commented Jan 16, 2024

Pull Request Test Coverage Report for Build 7543867449

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 85.246%

Totals Coverage Status
Change from base Build 7534439641: 0.0%
Covered Lines: 3630
Relevant Lines: 4140

💛 - Coveralls

Comment on lines -128 to -129
width: '90%',
marginHorizontal: '5%',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing is now handled by the modal. Spacing on the modal was necessary to achieve the same style while also setting a background to avoid the RTCView warning.

@@ -152,7 +149,6 @@ const defaultStyles = createStyles('SearchRecipientsModal', (theme) => {
marginVertical: 16,
marginRight: 16,
height: 35,
width: 55,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the upgrade this was causing the text to be truncated.

@@ -32,7 +32,7 @@ export const TraceLine = (props: Props) => {
const yDomain = [domainMin, domainMax] as [number, number];

const trend = useMemo(() => {
if (showTrend) {
if (showTrend && data.length > 1) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only show the trend if it has 2+ points. A single point produces a flat trend which isn't meaningful and 0 points causes a NaN error because of division by zero.

@@ -35,6 +35,7 @@ export function ScreenSurface({
const defaultStyles = createStyles('ScreenSurface', (theme) => ({
surfaceView: {
flex: 1,
height: '100%',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the upgrade a react native paper surface must have a height set to flex properly.

@@ -56,7 +56,7 @@ export const BottomNavigationBar = ({
? options.tabBarLabel
: options.title !== undefined
? options.title
: route.title;
: route.name;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the upgrade the title property was removed. We now default to name, however, in practice this should never happen.

@@ -122,8 +122,7 @@ export function MessageScreen<ParamList extends ParamListBase>({
<View
style={{
flexDirection: 'row',
flex: 1,
maxWidth: 60,
width: 60,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the upgrade the message avatars did not display correctly. Setting a static width solved this problem.

@sternetj sternetj merged commit 1a18ff9 into master Jan 17, 2024
3 checks passed
@sternetj sternetj deleted the fix-warnings branch January 17, 2024 15:36
Copy link

🎉 This PR is included in version 11.11.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot added the released Has been released to the package repository (NPM, etc) label Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Has been released to the package repository (NPM, etc)
Projects
None yet
3 participants