Skip to content

Design Kits issues #2836

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

Merged
merged 6 commits into from
Dec 6, 2023
Merged

Design Kits issues #2836

merged 6 commits into from
Dec 6, 2023

Conversation

ethanshar
Copy link
Collaborator

@ethanshar ethanshar commented Dec 5, 2023

Description

I have 2 fixes here

  • Our Colors.generateColorPalette api is memoized but it didn't handle well the options argument so we got mixed results
    This PR fix it by passing a custom resolver that takes the options into account
  • In TabBar I improved the key that rebuilds the components based on orientation, not it will rebuild it also based on labelColor & selectedLabelColor props
    The reason we rebuild it is because this code is heavily based on reanimated and require us to initialize the whole animation

Changelog

Fix Colors.generateColorPalette memoized result (handle cache properly)
Fix TabController.TabBar not responding to change in labelColor/selectedLabelColor

Additional info

@ethanshar ethanshar requested a review from Inbal-Tish December 5, 2023 16:24
@ethanshar ethanshar changed the title Add proper cache resolver to our memoize generatePalette method Design Kits issues Dec 6, 2023
@ethanshar ethanshar added the Important for Next Release PR that must be included in the release version label Dec 6, 2023
@@ -111,7 +111,7 @@ describe('style/Colors', () => {
uut.getColorTint('#3F88C5', 20);
uut.getColorTint('#3F88C5', 50);
uut.getColorTint('#3F88C5', 70);
const cachedPalette = uut.generateColorPalette.cache.get('#3F88C5');
const cachedPalette = uut.generateColorPalette.cache.get('#3F88C5_');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the underscore in the hax string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@Inbal-Tish
Copy link
Collaborator

@ethanshar Approved. I left a small comment

@ethanshar ethanshar enabled auto-merge (squash) December 6, 2023 08:10
@ethanshar ethanshar merged commit f420d03 into master Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Important for Next Release PR that must be included in the release version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants