fix(ThemeProvider): update theme with prop change

[arpitBhalla]

Related #3665

[codecov]

Codecov Report

Merging #3732 (a06a171) into next (a13b22b) will increase coverage by 0.38%.
The diff coverage is 100.00%.

❗ Current head a06a171 differs from pull request most recent head 52f3b72. Consider uploading reports for the commit 52f3b72 to get more accurate results

@@            Coverage Diff             @@
##             next    #3732      +/-   ##
==========================================
+ Coverage   79.47%   79.85%   +0.38%     
==========================================
  Files          87       87              
  Lines        1822     1822              
  Branches      805      804       -1     
==========================================
+ Hits         1448     1455       +7     
+ Misses        369      362       -7     
  Partials        5        5              

Impacted Files	Coverage Δ
packages/base/src/Avatar/Avatar.tsx	100.00% <ø> (ø)
packages/themed/src/config/ThemeProvider.tsx	76.59% <100.00%> (+12.76%)	⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[Nantris]

Thanks again for all your efforts @arpitBhalla. I think that the ThemeProvider changes seem good to go in my view.

If merged, it should resolve: #3655 and #3665

[Nantris]

@arpitBhalla I just wanted to thank you again for your great work. Thanks so much for what you do!

[Nantris]

@arpitBhalla is it planned to merge this? It's seemed ready to go for many months but it still isn't merged, not to mention released. Could you provide some clarity?

[Nantris]

Friendly bump.

I think the code changes look good. I noticed there's been no new release candidate for a while. Is it planned to release one soon?

Thank you again for all of your hard work, and please don't mistake my bump for impatience.

@arpitBhalla

[netlify]

✅ Deploy Preview for react-native-elements ready!

Name	Link
🔨 Latest commit	52f3b72
🔍 Latest deploy log	https://app.netlify.com/sites/react-native-elements/deploys/64802f94ac33b90008f751c5
😎 Deploy Preview	https://deploy-preview-3732--react-native-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[markrickert]

This PR broke my updateTheme() functionality when upgrading from rc.7 to rc.8.

I'm using a hook to determine if fonts should be scaled in the app:

// These constants determine mow much bigger the font size should get based on the user's
// accessibility settings. Even if they turn the dial all the way to 11, we will only ever
// scale the fonts by these factors. This is to prevent the font size from getting too large
// and completely breaking the layout.
export const MAX_FONT_SCALE = 1.2;
export const MIN_FONT_SCALE = 0.8;

// Returns fontScaling props for Text and TextInput components
export const useFontScaling = (): Partial<TextProps> => {
  const { preferenceStore } = useStores();

  const fontScaling: Partial<TextProps> = React.useMemo(() => {
    const afs = preferenceStore?.allowFontScaling || false;
    return {
      minimumFontScale: afs ? MIN_FONT_SCALE : 1, // This prevents the font from getting too small.
      maxFontSizeMultiplier: afs ? MAX_FONT_SCALE : 1, // This prevents the font from getting too big.
      allowFontScaling: afs, // This allows the font to be scaled.
    };
  }, [preferenceStore?.allowFontScaling]);

  return fontScaling;
};

Then whenever it changes, i try and update the current theme:

const { updateTheme } = useTheme();
const fontScalingProps = useFontScaling();

React.useEffect(()=>{
  updateTheme({
    components: {
      Text: {
        ...fontScalingProps,
      },
      // Other rneui components as necessary
    }
  });
},[fontScalingProps]);

rc.8 completely broke this functionality because of this PR. I manually reverted changes in the ThemeProvider.tsx and the updates resumed working as intended.

Am I updating the theme incorrectly or is this an actual regression? Some guidance would be appreciated, thanks!

[Nantris]

@markrickert it happens because you need to maintain object equality for the theme object after this PR (which if I'm not mistaken is how version 3.x handled it)

An approach to handling this might be like:

const [theme, setTheme] = React.useState({
    components: {
      Text: {
        ...fontScalingProps,
      },
      // Other rneui components as necessary
    }
});

React.useEffect(() => {
    const updatedTheme = {
        components: {
            Text: {
                ...fontScalingProps,
            },
            // Other rneui components as necessary
        }
    };
    
    setTheme(updatedTheme);
    updateTheme(updatedTheme);
}, [fontScalingProps]);