Add drawing lines and opacity on rectangle drawing plugin #1899
Conversation
woung717
changed the title
|
@SlicedSilver Could you review this PR? Thanks! |
SlicedSilver
left a comment
SlicedSilver
left a comment
There was a problem hiding this comment.
Thanks for the PR. It certainly improves the example.
I have a minor suggestion that it would be neater if each tool had it's own file/s, and ideally that they would be implementing a new interface like IDrawingTool so that it is easier to extend or add more tools in the future by implementing that interface and adding the definition to the root DrawingTools class. However this isn't required for this PR.
|
@SlicedSilver Thank you for taking the time to review this PR. Seems I missed some parts. I'll let you know when it's been corrected. |
| export interface IDrawingTool { | ||
| _chart: IChartApi; | ||
| _series: ISeriesApi<SeriesType>; | ||
| _defaultOptions: any; | ||
| _points: Point[]; | ||
| _drawing: boolean; | ||
| _onDrawingCompleteCallback?: () => void; | ||
|
|
||
| _clickHandler: (param: MouseEventParams) => void; | ||
| _moveHandler: (param: MouseEventParams) => void; | ||
| _dblClickHandler: (param: MouseEventParams) => void; | ||
|
|
||
| _onClick(param: MouseEventParams): void; | ||
| _onMouseMove(param: MouseEventParams): void; | ||
| _onDblClick(param: MouseEventParams): void; | ||
| _addPoint(p: Point): void; | ||
|
|
||
| options: any; | ||
| remove(): void; | ||
| startDrawing(): void; | ||
| stopDrawing(): void; | ||
| isDrawing(): boolean; | ||
| } | ||
|
|
||
| export interface IShape { | ||
| _id: string; | ||
| _p1: Point; | ||
| _p2: Point; | ||
| _options: any; | ||
| updateAllViews(): void; | ||
| priceAxisViews(): ISeriesPrimitiveAxisView[]; | ||
| timeAxisViews(): ISeriesPrimitiveAxisView[]; | ||
| paneViews(): IPrimitivePaneView[]; | ||
| applyOptions(options: any): void; | ||
| hitTest(x: Coordinate, y: Coordinate): PrimitiveHoveredItem | null; | ||
| } | ||
|
|
||
| export interface IPreviewShape { | ||
| updateEndPoint(p: Point): void; | ||
| } No newline at end of file |
There was a problem hiding this comment.
@SlicedSilver I need your opinion here. I added interfaces (IDrawingTool, IShape, IPreviewShape) and made each shape class implement them. However, I'm not sure if it would be better to have IShape and IPreviewShape for Line/Rectangle and PreviewLine/PreviewRectangle too, respectively.
Do you think it constraints how shapes can be implemented too much?
There was a problem hiding this comment.
I would suggest that the interfaces don't include the private fields (with the leading underscore). IShape looks like it is essentially describing the Primitive interface so you could probably extend that instead and remove the duplications (or use a Pick helper type provided by typescript). IPreviewShape looks good, but perhaps it should extend IShape for completeness.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the rectangle drawing tool by adding line drawing functionality and opacity control, while renaming it to "Drawing Tools" to reflect its expanded capabilities.
- Refactored the rectangle drawing plugin into a more modular drawing tools system
- Added line drawing capability alongside existing rectangle functionality
- Implemented opacity control through a slider in the toolbar
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rectangle-drawing-tool.ts | Removed original rectangle-only implementation |
| drawing-tools/types.ts | Added shared type definitions for Point and ViewPoint |
| drawing-tools/rectangle.ts | Refactored rectangle drawing with improved modularity and hit testing |
| drawing-tools/options.ts | Created centralized options configuration for both tools |
| drawing-tools/line.ts | Implemented new line drawing functionality |
| drawing-tools/interfaces.ts | Defined common interfaces for drawing tools and shapes |
| drawing-tools/drawing-tools.ts | Main orchestrator class managing both rectangle and line tools |
| example files | Updated to use new DrawingTools class and reflect naming changes |
| index.html | Updated navigation link to point to renamed plugin |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
2 participants
Type of PR: enhancement
PR checklist:
Overview of change:
In addition to the existing rectangle drawing tool, the ability to draw a line and adjust the opacity of shapes has been added.
Is there anything you'd like reviewers to focus on?
Since the previous file and class name was "Rectangle Drawing" plugin, I changed it to "Drawing Tools". This include directory path name (website path too).
If you need to see changes, compare the previous versions of rectangle-drawing-tool.ts and the new drawing-tools.ts.