-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Chore]: Technical: add types for editor component #1797
Conversation
f0afc76
to
950312f
Compare
src/components/editor/editor.tsx
Outdated
layersToRender: Record<string, Layer>; | ||
index: number; | ||
className: string; // ?? | ||
classnames: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why need classnames
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced
src/components/editor/editor.tsx
Outdated
|
||
export default function EditorFactory( | ||
FeatureActionPanel: ComponentType<FeatureActionPanelProps> | ||
): ComponentType<EditorProps> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be Reac.Component<EditorProps>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced by React.ComponentClass
src/components/editor/editor.tsx
Outdated
|
||
export default function EditorFactory( | ||
FeatureActionPanel: ComponentType<FeatureActionPanelProps> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be React.FC<FeatureActionPanelProps>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
onClose: () => void; | ||
} | ||
|
||
export function PureFeatureActionPanelFactory(): ComponentType<FeatureActionPanelProps> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const FeatureActionPanel: React.FC<FeatureActionPanelProps>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
export interface FeatureActionPanelProps { | ||
className?: string; | ||
datasets: Datasets; | ||
selectedFeature: Feature; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
selectedFeature
can be null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -124,9 +132,9 @@ FeatureActionPanelFactory.deps = PureFeatureActionPanelFactory.deps; | |||
export default function FeatureActionPanelFactory() { | |||
const PureFeatureActionPanel = PureFeatureActionPanelFactory(); | |||
|
|||
const ClickOutsideFeatureActionPanel = props => { | |||
const ClickOutsideFeatureActionPanel = (props: FeatureActionPanelProps) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const ClickOutsideFeatureActionPanel: React.FC<FeatureActionPanelProps>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com> Signed-off-by: Evgeny Zhgulev <evgeny.zhgulev@actionengine.com>
Signed-off-by: Evgeny Zhgulev <evgeny.zhgulev@actionengine.com>
ed3134f
to
50265d0
Compare
LGTM |
src/components/editor/editor.tsx
Outdated
state = { | ||
showActions: false, | ||
lastPosition: null | ||
lastPosition: {x: 0, y: 0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep this as null
in feature-action-panel
add
if (!position) {
return null
}
return (
<StyledActionsLayer
className={classnames('feature-action-panel', className)}
style={{
top: `${position.y + LAYOVER_OFFSET}px`,
left: `${position.x + LAYOVER_OFFSET}px`
}}
>
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
FeatureActionPanel.defaultProps = { | ||
position: {} | ||
position: {x: 0, y: 0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make default position to be null. and return null
in render if position
is null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
also fixed a test for FeatureActionPanel, plz check it out
Signed-off-by: Evgeny Zhgulev <evgeny.zhgulev@actionengine.com>
Signed-off-by: Evgeny Zhgulev <evgeny.zhgulev@actionengine.com>
Signed-off-by: Daria Terekhova daria.terekhova@actionengine.com