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.

Sign up for GitHub

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

Allow iOS PlatformColor strings to be ObjC or Swift UIColor selectors #28703

Closed
wants to merge 1 commit into from
Closed

Allow iOS PlatformColor strings to be ObjC or Swift UIColor selectors #28703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 50 additions & 50 deletions RNTester/js/examples/PlatformColor/PlatformColorExample.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,94 +22,94 @@ function PlatformColorsExample() {
colors = [
// https://developer.apple.com/documentation/uikit/uicolor/ui_element_colors
// Label Colors
{label: 'labelColor', color: PlatformColor('labelColor')},
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to keep any examples that include the Color suffix? And / or maybe have an automated test that verifies both ways work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I said in the Test notes, there still are ObjC examples later in this file and in unit tests

Copy link
Member

Choose a reason for hiding this comment

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

Ah, whoops. I should read better :D

{label: 'label', color: PlatformColor('label')},
{
label: 'secondaryLabelColor',
color: PlatformColor('secondaryLabelColor'),
label: 'secondaryLabel',
color: PlatformColor('secondaryLabel'),
},
{
label: 'tertiaryLabelColor',
color: PlatformColor('tertiaryLabelColor'),
label: 'tertiaryLabel',
color: PlatformColor('tertiaryLabel'),
},
{
label: 'quaternaryLabelColor',
color: PlatformColor('quaternaryLabelColor'),
label: 'quaternaryLabel',
color: PlatformColor('quaternaryLabel'),
},
// Fill Colors
{label: 'systemFillColor', color: PlatformColor('systemFillColor')},
{label: 'systemFill', color: PlatformColor('systemFill')},
{
label: 'secondarySystemFillColor',
color: PlatformColor('secondarySystemFillColor'),
label: 'secondarySystemFill',
color: PlatformColor('secondarySystemFill'),
},
{
label: 'tertiarySystemFillColor',
color: PlatformColor('tertiarySystemFillColor'),
label: 'tertiarySystemFill',
color: PlatformColor('tertiarySystemFill'),
},
{
label: 'quaternarySystemFillColor',
color: PlatformColor('quaternarySystemFillColor'),
label: 'quaternarySystemFill',
color: PlatformColor('quaternarySystemFill'),
},
// Text Colors
{
label: 'placeholderTextColor',
color: PlatformColor('placeholderTextColor'),
label: 'placeholderText',
color: PlatformColor('placeholderText'),
},
// Standard Content Background Colors
{
label: 'systemBackgroundColor',
color: PlatformColor('systemBackgroundColor'),
label: 'systemBackground',
color: PlatformColor('systemBackground'),
},
{
label: 'secondarySystemBackgroundColor',
color: PlatformColor('secondarySystemBackgroundColor'),
label: 'secondarySystemBackground',
color: PlatformColor('secondarySystemBackground'),
},
{
label: 'tertiarySystemBackgroundColor',
color: PlatformColor('tertiarySystemBackgroundColor'),
label: 'tertiarySystemBackground',
color: PlatformColor('tertiarySystemBackground'),
},
// Grouped Content Background Colors
{
label: 'systemGroupedBackgroundColor',
color: PlatformColor('systemGroupedBackgroundColor'),
label: 'systemGroupedBackground',
color: PlatformColor('systemGroupedBackground'),
},
{
label: 'secondarySystemGroupedBackgroundColor',
color: PlatformColor('secondarySystemGroupedBackgroundColor'),
label: 'secondarySystemGroupedBackground',
color: PlatformColor('secondarySystemGroupedBackground'),
},
{
label: 'tertiarySystemGroupedBackgroundColor',
color: PlatformColor('tertiarySystemGroupedBackgroundColor'),
label: 'tertiarySystemGroupedBackground',
color: PlatformColor('tertiarySystemGroupedBackground'),
},
// Separator Colors
{label: 'separatorColor', color: PlatformColor('separatorColor')},
{label: 'separator', color: PlatformColor('separator')},
{
label: 'opaqueSeparatorColor',
color: PlatformColor('opaqueSeparatorColor'),
label: 'opaqueSeparator',
color: PlatformColor('opaqueSeparator'),
},
// Link Color
{label: 'linkColor', color: PlatformColor('linkColor')},
{label: 'link', color: PlatformColor('link')},
// Nonadaptable Colors
{label: 'darkTextColor', color: PlatformColor('darkTextColor')},
{label: 'lightTextColor', color: PlatformColor('lightTextColor')},
{label: 'darkText', color: PlatformColor('darkText')},
{label: 'lightText', color: PlatformColor('lightText')},
// https://developer.apple.com/documentation/uikit/uicolor/standard_colors
// Adaptable Colors
{label: 'systemBlueColor', color: PlatformColor('systemBlueColor')},
{label: 'systemBrownColor', color: PlatformColor('systemBrownColor')},
{label: 'systemGreenColor', color: PlatformColor('systemGreenColor')},
{label: 'systemIndigoColor', color: PlatformColor('systemIndigoColor')},
{label: 'systemOrangeColor', color: PlatformColor('systemOrangeColor')},
{label: 'systemPinkColor', color: PlatformColor('systemPinkColor')},
{label: 'systemPurpleColor', color: PlatformColor('systemPurpleColor')},
{label: 'systemRedColor', color: PlatformColor('systemRedColor')},
{label: 'systemTealColor', color: PlatformColor('systemTealColor')},
{label: 'systemYellowColor', color: PlatformColor('systemYellowColor')},
{label: 'systemBlue', color: PlatformColor('systemBlue')},
{label: 'systemBrown', color: PlatformColor('systemBrown')},
{label: 'systemGreen', color: PlatformColor('systemGreen')},
{label: 'systemIndigo', color: PlatformColor('systemIndigo')},
{label: 'systemOrange', color: PlatformColor('systemOrange')},
{label: 'systemPink', color: PlatformColor('systemPink')},
{label: 'systemPurple', color: PlatformColor('systemPurple')},
{label: 'systemRed', color: PlatformColor('systemRed')},
{label: 'systemTeal', color: PlatformColor('systemTeal')},
{label: 'systemYellow', color: PlatformColor('systemYellow')},
// Adaptable Gray Colors
{label: 'systemGrayColor', color: PlatformColor('systemGrayColor')},
{label: 'systemGray2Color', color: PlatformColor('systemGray2Color')},
{label: 'systemGray3Color', color: PlatformColor('systemGray3Color')},
{label: 'systemGray4Color', color: PlatformColor('systemGray4Color')},
{label: 'systemGray5Color', color: PlatformColor('systemGray5Color')},
{label: 'systemGray6Color', color: PlatformColor('systemGray6Color')},
{label: 'systemGray', color: PlatformColor('systemGray')},
{label: 'systemGray2', color: PlatformColor('systemGray2')},
{label: 'systemGray3', color: PlatformColor('systemGray3')},
{label: 'systemGray4', color: PlatformColor('systemGray4')},
{label: 'systemGray5', color: PlatformColor('systemGray5')},
{label: 'systemGray6', color: PlatformColor('systemGray6')},
];
} else if (Platform.OS === 'android') {
colors = [
Expand Down
21 changes: 19 additions & 2 deletions React/Base/RCTConvert.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ +(type)type : (id)json \
{
static NSDictionary<NSString *, NSDictionary *> *colorMap = nil;
if (colorMap == nil) {
colorMap = @{
NSMutableDictionary<NSString *, NSDictionary *> *map = [@{
// https://developer.apple.com/documentation/uikit/uicolor/ui_element_colors
// Label Colors
@"labelColor" : @{
Expand Down Expand Up @@ -729,7 +729,22 @@ +(type)type : (id)json \
// iOS 13.0
RCTFallbackARGB : @(0xFFf2f2f7)
},
} mutableCopy];
// The color names are the Objective-C UIColor selector names,
// but Swift selector names are valid as well, so make aliases.
static NSString *const RCTColorSuffix = @"Color";
NSMutableDictionary<NSString *, NSDictionary *> *aliases = [NSMutableDictionary new];
for (NSString *objcSelector in map) {
RCTAssert([objcSelector hasSuffix:RCTColorSuffix], @"A selector in the color map did not end with the suffix Color.");
NSMutableDictionary *entry = [map[objcSelector] mutableCopy];
RCTAssert([entry objectForKey:RCTSelector] == nil, @"Entry should not already have an RCTSelector");
NSString *swiftSelector = [objcSelector substringToIndex:[objcSelector length] - [RCTColorSuffix length]];
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this code could have been slightly simpler/faster if it had been reversed, i.e. manually add them all without the Color suffix and then alias them by appending the suffix. Not a huge deal [on its own], though, as this map is cached.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If reversed, it would mean having to add an explicit RCTSelector for each entry -- which is more error prone for maintenance. I like it this way because the table is more truthy to ObjC which is the native language of the RN implementation itself.

entry[RCTSelector] = objcSelector;
aliases[swiftSelector] = entry;
}
[map addEntriesFromDictionary:aliases];
#if DEBUG
[map addEntriesFromDictionary:@{
// The follow exist for Unit Tests
@"unitTestFallbackColor" : @{RCTFallback : @"gridColor"},
@"unitTestFallbackColorIOS" : @{RCTFallback : @"blueColor"},
Expand All @@ -743,9 +758,11 @@ +(type)type : (id)json \
RCTIndex : @1,
RCTFallback : @"controlAlternatingRowBackgroundColors"
},
}];
#endif
};
colorMap = [map copy];
}

return colorMap;
}

Expand Down