Skip to content

Conversation

wilmxre
Copy link
Contributor

@wilmxre wilmxre commented Feb 14, 2024

Hello there. I just added two new properties to the actions object: customIcon and customIconColor. There are some things that are good to know:

  • when both systemIcon and customIcon is provided, customIcon will overwrite the icon specified in systemIcon
  • customIconColor only applies to customIcon, i tried applying it to systemIcon too, but with no success
  • you have to call processColor() from react-native, when you add the customIconColor prop (ex. actions={[{ customIconColor: processColor('#385858') }]}). as i am not too familiar with objective-c, i don't know how can you provide the string specified on the JS side as an integer to RCTConvert, which should give us an UIColor after converting said integer. in my personal project i don't use processColor(), as i implemented the hex conversion on the native side with:
// credits to @itsjustcon on GitHub for this snippet

+ (UIColor *) colorFromHexCode:(NSString *)hexString {
   NSString *cleanString = [hexString stringByReplacingOccurrencesOfString:@"#" withString:@""];
   if([cleanString length] == 3) {
       cleanString = [NSString stringWithFormat:@"%@%@%@%@%@%@",
                       [cleanString substringWithRange:NSMakeRange(0, 1)],[cleanString substringWithRange:NSMakeRange(0, 1)],
                       [cleanString substringWithRange:NSMakeRange(1, 1)],[cleanString substringWithRange:NSMakeRange(1, 1)],
                       [cleanString substringWithRange:NSMakeRange(2, 1)],[cleanString substringWithRange:NSMakeRange(2, 1)]];
   }
   if([cleanString length] == 6) {
       cleanString = [cleanString stringByAppendingString:@"ff"];
   }
   
   unsigned int baseValue;
   [[NSScanner scannerWithString:cleanString] scanHexInt:&baseValue];
   
   float red = ((baseValue >> 24) & 0xFF)/255.0f;
   float green = ((baseValue >> 16) & 0xFF)/255.0f;
   float blue = ((baseValue >> 8) & 0xFF)/255.0f;
   float alpha = ((baseValue >> 0) & 0xFF)/255.0f;
   
   return [UIColor colorWithRed:red green:green blue:blue alpha:alpha];
}

but maybe the simplistic native side code would be too bloated with this.

Anyways, please tell me if something needs to be modified or if there is a better way to implement this, i take advices happily.

@wilmxre wilmxre changed the title Add customIcon and customIconName properties to the action object feat(iOS): add customIcon and customIconName properties to the action object Feb 14, 2024
@mpiannucci
Copy link
Owner

Thank you for this! I will review soon and be in touch!

@wilmxre
Copy link
Contributor Author

wilmxre commented Feb 16, 2024

okay. let me know if there is a better way converting the color. as i understood a direct prop of a view can be converted using this macro: RCT_CUSTOM_VIEW_PROPERTY. but i didn't find a way to mark action.customColor with this, as it is a prop of the action object under the actions prop

@mpiannucci
Copy link
Owner

Before merging I want to make sure there is not a better way to pass the color to iOS like we do on android.

I also am thinking about the API, since Android supports this exact same workflow so they should probably use the same props to do so.

I think icon and iconColor may work for this and leave system icon as iOS only for now, while the others refer directly to provided assets.

Thoughts?

@wilmxre
Copy link
Contributor Author

wilmxre commented Feb 17, 2024

yeah, now the passing of the color is a bit messy for iOS with the whole processColor, but as a last resort we could just use the color converter, that i mentioned, which is just a few lines of code and works really well for hex colors. but then it doesn't support react-native style colors like aliceblue and transparent etc, so idk. it would be nice if we could use RCTConvert and process the color on the native side. let me know if you found a good solution for this

sounds good to me, i thought about it too, but didn't want to mess with the previously set prop names too much, as it will break other people's code with the refactoring. but i can change the names for both iOS and Android to just icon and iconColor. i will come up with a new commit and we can figure out the next steps

@wilmxre
Copy link
Contributor Author

wilmxre commented Feb 17, 2024

okay i refactored the prop names and made some modifications to the README. in the type declarations i marked iconColor with string only, if we can process the color somehow in iOS on the native side it should stay like that. the example app is unchanged now, when we will have a mergeable PR we can change that to fit the up to date API

@wilkinsonj
Copy link

This is clean work, nice job

@mpiannucci
Copy link
Owner

This is awesome work.

For processColor what if the user passed in the color as they normally would and processColor was called in index.ts before the prop was sent down to ios. Something like this?

index.js

const ContextMenu = (props) => {
  const iconColor = props.iconColor ? processColor(props.iconColor) : undefined;
  return (
    <NativeContextMenu {...props} iconColor={iconColor}>
      {props.children}
      {props.preview != null && Platform.OS === 'ios' ? (
        <View style={styles.preview} nativeID="ContextMenuPreview">{props.preview}</View>
      ) : null}
    </NativeContextMenu>
  );
};

@wilmxre
Copy link
Contributor Author

wilmxre commented Feb 20, 2024

okay, this should be good enough. i added this change, but checked for iOS, because on Android i think you can pass a react-native color normally. right now i won't have much time, but later at night i will test it and change the example app to fit the new changes :)

@mpiannucci
Copy link
Owner

All in all, this is great. Thanks for the awesome contribution

@mpiannucci mpiannucci merged commit bea0a1c into mpiannucci:main Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants