Add oportunity to set penColor via prop in Android#5
Conversation
|
@keshavkaul why not merged? |
|
@CandyOgre I had a busy fortnight, didn't get a chance to work on this. Will look into the PR in coming days. Thanks. |
kaykaul
left a comment
There was a problem hiding this comment.
@CandyOgre I have reviewed the PR and added comments. Do check, thanks!
| 1. `selectedTool` - Set the tool id to be selected. | ||
| 2. `localSourceImagePath` - Local file path of the image. | ||
| 3. `onSaveSketch(saveArgs)` - Callback when saving is complete. | ||
| 2. `penColor` - Set color for pen (currently only for Android). |
There was a problem hiding this comment.
I would change the prop name from penColor to toolColor because I'm thinking of pen as one of the many tools with their own style of sketching. For example, a pencil tool might have the effect of pencil on paper. But that's currently not in the scope. But visualizing a tool having properties like color, thickness etc. is a good start.
| private static final String RN_PACKAGE = "RNSketchView"; | ||
|
|
||
| private static final String PROPS_SELECTED_TOOL = "selectedTool"; | ||
| private static final String PROPS_PEN_COLOR = "penColor"; |
| } | ||
|
|
||
| public void setPenColor(int toolColor) { | ||
| currentTool = new PenSketchTool(this, toolColor); |
There was a problem hiding this comment.
should check if current tool implements ToolColor interface, and calls the setToolColor on the current tool if it does.
There was a problem hiding this comment.
No need to create another PenSketchTool. I have already implemented setting pen color in the PenSketchTool.
| paint.setStrokeCap(Paint.Cap.ROUND); | ||
| } | ||
|
|
||
| public PenSketchTool(View touchView, int color) { |
| SketchView.propTypes = { | ||
| ...View.propTypes, // include the default view properties | ||
| selectedTool: PropTypes.number, | ||
| penColor: PropTypes.number, |
| } | ||
|
|
||
| @ReactProp(name = PROPS_PEN_COLOR) | ||
| public void setPenColor(SketchViewContainer viewContainer, @NonNull Integer color) { |
There was a problem hiding this comment.
change method name to setToolColor
No description provided.