post merge fixes and spaces tweaks #4475
Conversation
WalkthroughThe recent changes encompass updates to component properties, function parameters, and type definitions across various files. Key modifications include the addition of new properties to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
imanjra
force-pushed
the
bugfix/python-panels-im-x1
branch
from
3e81fc8
to
6f2ffe9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (5)
app/packages/operators/src/operators.ts (5)
Line range hint
91-91: ### Default Parameter Order Issue
The default parameters in the constructors ofInvocationRequest,Executor, andExecutionContextare incorrectly placed before required parameters. This can lead to issues where the default parameters cannot be omitted at the call site without specifying all subsequent parameters, which is counterintuitive.Consider rearranging the parameters so that all default parameters follow the required ones. This change will make the function signatures clearer and more predictable.
- constructor(public operatorURI: string, public params: unknown = {}, public options?: OperatorExecutorOptions) + constructor(public operatorURI: string, public options?: OperatorExecutorOptions, public params: unknown = {}) - constructor(public params: object = {}, public _currentContext: RawContext, public hooks: object = {}, public executor: Executor = null) + constructor(public _currentContext: RawContext, public hooks: object = {}, public executor: Executor = null, public params: object = {})Also applies to: 155-155, 156-156, 759-759
Line range hint
213-213: ### Simplify Boolean Expressions
The use of ternary operators for boolean values in theOperatorConfigconstructor is unnecessary. You can simplify the code by directly assigning the boolean values without using ternary operators. This will make the code cleaner and easier to read.- this.onStartup = options.onStartup || false; - this.onDatasetOpen = options.onDatasetOpen || false; + this.onStartup = Boolean(options.onStartup); + this.onDatasetOpen = Boolean(options.onDatasetOpen);Also applies to: 215-215
Line range hint
268-268: ### Suggest Using Optional Chaining
There are several places in the code where optional chaining could be used to make the code more concise and safe. This modern JavaScript feature helps prevent runtime errors due to attempts to access properties onnullorundefined.- if (result && result.error) { + if (result?.error) { - if (errorBody && errorBody.kind === "Server Error") { + if (errorBody?.kind === "Server Error") { - if (typeAsJSON && typeAsJSON.error) { + if (typeAsJSON?.error) { - if (operatorURI && operatorURI.includes(HASH)) { + if (operatorURI?.includes(HASH)) { - if (currentContext && currentContext.selectedSamples) { + if (currentContext?.selectedSamples) {Also applies to: 560-560, 742-742, 844-844, 893-893
Line range hint
305-305: ### Avoid Usingvoidin Union Types
The use ofvoidin union types can be confusing and misleading. It's better to useundefinedin such cases to make the intentions clear and the code more predictable.- public resolvePlacement(): Promise<void | types.Placement> { + public resolvePlacement(): Promise<undefined | types.Placement> {
Line range hint
313-313: ### Static Method Context Issue
Usingthisin a static context can lead to confusion as it does not behave as it might be expected in an instance method context. It's recommended to use the class name instead ofthisto refer to static methods or properties.- const operator = this.fromJSON(json); + const operator = Operator.fromJSON(json);
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- app/packages/components/src/components/Popout/index.tsx (2 hunks)
- app/packages/operators/src/operators.ts (1 hunks)
- app/packages/operators/src/useCustomPanelHooks.ts (2 hunks)
- app/packages/operators/src/usePanelEvent.ts (1 hunks)
- app/packages/spaces/src/components/AddPanelButton.tsx (2 hunks)
Additional context used
Path-based instructions (5)
app/packages/operators/src/usePanelEvent.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/components/src/components/Popout/index.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/spaces/src/components/AddPanelButton.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/operators/src/useCustomPanelHooks.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/operators/src/operators.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
Biome
app/packages/operators/src/useCustomPanelHooks.ts
[error] 39-39: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 40-40: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
app/packages/operators/src/operators.ts
[error] 91-91: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 155-155: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 156-156: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 213-213: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
[error] 215-215: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
[error] 268-268: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 305-305: void is confusing inside a union type. (lint/suspicious/noConfusingVoidType)
Unsafe fix: Use undefined instead.
[error] 313-313: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 560-560: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 742-742: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 759-759: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 844-844: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 893-893: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Additional comments not posted (4)
app/packages/operators/src/usePanelEvent.ts (1)
32-32: The update to passcallbackas an object inexecuteOperatoraligns with the PR objectives and improves the clarity of parameter passing.app/packages/components/src/components/Popout/index.tsx (1)
12-12: The addition ofpopoutPropstoPopoutPropsand its application inPopoutDivenhances component flexibility and customizability. Ensure to document these changes to guide developers on how to use the new properties effectively.Also applies to: 36-36
app/packages/spaces/src/components/AddPanelButton.tsx (1)
49-58: The updated styling and configuration of thePopoutcomponent to includemaxHeight,overflow, andpopoutPropseffectively make the add panel popover scrollable, enhancing user experience for scenarios with many panels. Consider conducting user testing to verify the visual and functional aspects of these changes.app/packages/operators/src/useCustomPanelHooks.ts (1)
16-26: The change in property types fromFunctiontostringinCustomPanelPropssuggests a significant shift in usage. Ensure to document or comment on these changes to clarify how developers should now use these properties.
| uri: string, | ||
| params: unknown = {}, | ||
| options?: OperatorExecutorOptions | ||
| ) { | ||
| const { operatorURI, params } = resolveOperatorURIWithMethod(uri, p); | ||
| const { operatorURI, params: computedParams } = resolveOperatorURIWithMethod( | ||
| uri, | ||
| params | ||
| ); | ||
| const resolvedOperatorURI = resolveOperatorURI(operatorURI); | ||
| const queue = getInvocationRequestQueue(); | ||
| const request = new InvocationRequest(resolvedOperatorURI, params); | ||
| queue.add(request, callback); | ||
| const request = new InvocationRequest( | ||
| resolvedOperatorURI, | ||
| computedParams, | ||
| options | ||
| ); | ||
| queue.add(request); |
There was a problem hiding this comment.
Tip
Codebase Verification
The script output shows multiple instances of executeOperator being called across various files. However, not all calls include the optional options parameter, which is necessary to match the updated function signature.
Instances that need attention:
app/packages/operators/src/useCustomPanelHooks.ts(Lines: 80, 111, 125, 145, 153)app/packages/operators/src/usePanelEvent.ts(Line: 32)app/packages/operators/src/built-in-operators.ts(Lines: 728, 786)app/packages/operators/src/operators.ts(Lines: 434, 537)
These lines should be reviewed and updated to ensure they include the optional options parameter as per the new function signature.
Analysis chain
Update to executeOperator Function Signature and Logic
The update to the executeOperator function to include an optional options parameter and the renaming of params to computedParams within the function body aligns with the PR objectives to make the third parameter explicitly for options. This change enhances the function's flexibility and clarity in handling operator execution options.
However, ensure that all calls to executeOperator throughout the codebase are updated to reflect this new signature to avoid runtime errors.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all function calls to `executeOperator` match the new signature.
# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type typescript $'executeOperator'
Length of output: 73
Script:
#!/bin/bash
# Description: Verify all function calls to `executeOperator` match the new signature.
# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type ts 'executeOperator'
Length of output: 3333
Script:
#!/bin/bash
# Description: Extract lines where `executeOperator` is called to verify the new function signature.
# Extract lines with `executeOperator` calls
rg --type ts 'executeOperator' -n | while read -r line; do
file=$(echo "$line" | cut -d: -f1)
lineno=$(echo "$line" | cut -d: -f2)
echo "File: $file, Line: $lineno"
sed -n "${lineno}p" "$file"
echo
done
Length of output: 20060
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (6)
app/packages/core/src/plugins/SchemaIO/components/Button.tsx (1)
5-5: Ensure destructuring ofvariantis necessary for further operations.While destructuring
variantdirectly frompropsis clean for current usage, consider future cases where more properties might be used fromprops.fiftyone/operators/types.py (5)
Line range hint
13-13: Remove unnecessary inheritance fromobject.- class BaseType(object): + class BaseType:
Line range hint
81-84: Simplify conditional assignment using a ternary operator.- if view is None: - view = View() - else: - view = view.clone() + view = View() if view is None else view.clone()
Line range hint
805-805: Remove unnecessary inheritance fromobject.- class View(object): + class View:
Line range hint
1383-1383: Remove unnecessary inheritance fromobject.- class Placement(object): + class Placement:
Line range hint
1852-1852: Remove unnecessary inheritance fromobject.- class ViewTargetOptions(object): + class ViewTargetOptions:
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- app/packages/core/src/plugins/SchemaIO/components/Button.tsx (1 hunks)
- app/packages/core/src/plugins/SchemaIO/components/ButtonView.tsx (2 hunks)
- app/packages/core/src/plugins/SchemaIO/components/IconButtonView.tsx (4 hunks)
- app/packages/operators/src/useCustomPanelHooks.ts (5 hunks)
- fiftyone/operators/types.py (7 hunks)
Additional context used
Path-based instructions (4)
app/packages/core/src/plugins/SchemaIO/components/Button.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/core/src/plugins/SchemaIO/components/ButtonView.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/core/src/plugins/SchemaIO/components/IconButtonView.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/operators/src/useCustomPanelHooks.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
Biome
app/packages/operators/src/useCustomPanelHooks.ts
[error] 39-39: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 40-40: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
Ruff
fiftyone/operators/types.py
13-13: Class
BaseTypeinherits fromobject(UP004)Remove
objectinheritance
81-84: Use ternary operator
view = View() if view is None else view.clone()instead ofif-else-block (SIM108)Replace
if-else-block withview = View() if view is None else view.clone()
805-805: Class
Viewinherits fromobject(UP004)Remove
objectinheritance
1143-1143: Dictionary key literal
"href"repeated (F601)Remove repeated key literal
"href"
1383-1383: Class
Placementinherits fromobject(UP004)Remove
objectinheritance
1852-1852: Class
ViewTargetOptionsinherits fromobject(UP004)Remove
objectinheritance
Additional comments not posted (10)
app/packages/core/src/plugins/SchemaIO/components/Button.tsx (1)
8-8: Conditional color assignment based onvariantprop.This change allows the button color to dynamically adjust based on the
variantprop, enhancing UI flexibility.app/packages/core/src/plugins/SchemaIO/components/ButtonView.tsx (2)
1-1: Review of new imports and their integration.The new imports are correctly integrated and used within the components. This ensures that the necessary functionalities are available for the components.
Also applies to: 2-2, 4-4, 6-6
19-19: Use ofvariantandhrefinBaseButtonView.The inclusion of
variantandhrefprops enhances the component's flexibility and functionality, allowing it to handle different types and behaviors of buttons.Also applies to: 23-23
app/packages/core/src/plugins/SchemaIO/components/IconButtonView.tsx (2)
4-4: Review of new imports and their integration.The new imports are correctly integrated and used within the component, ensuring that the necessary functionalities are available.
Also applies to: 8-8
13-13: Updates toIconButtonViewto handle various button styles and behaviors.The updates provide enhanced flexibility and customization for the
IconButtonViewcomponent, allowing it to handle different styles and behaviors based on thevariantandhrefprops.Also applies to: 20-20, 30-41, 51-51, 59-71, 77-79, 98-113
app/packages/operators/src/useCustomPanelHooks.ts (2)
16-26: UpdateCustomPanelPropsto use strings for event handlers.Changing the event handlers to strings likely aligns with a strategy to reference these handlers by identifiers rather than direct function calls, potentially simplifying configuration and integration with dynamic loading or remote execution scenarios.
57-57: Review ofuseCustomPanelHookslogic and its integration with the updated props.The logic within
useCustomPanelHookseffectively utilizes the updated string-based event handlers, ensuring that they are correctly invoked based on the component lifecycle and state changes. This integration supports dynamic and flexible panel behavior.Also applies to: 78-78, 85-85, 108-108, 113-123, 130-130, 138-151
fiftyone/operators/types.py (3)
Line range hint
255-305: Thebtnmethod has been updated to include a newhrefparameter, allowing buttons to navigate to URLs. This change aligns with modern web practices and enhances the flexibility of button components in the UI.
Line range hint
1124-1143: TheButtonclass now includes anhrefparameter, enabling navigation functionality directly from buttons. This is a useful feature for creating more interactive and connected UI components.
2037-2045: TheIconButtonViewclass has been updated to include anhrefparameter, allowing icon buttons to also perform navigation tasks. This enhancement makes the UI elements more versatile and functional.
imanjra
force-pushed
the
bugfix/python-panels-im-x1
branch
from
c6d0052
to
4b7e5f7
Compare
imanjra
force-pushed
the
bugfix/python-panels-im-x1
branch
from
4b7e5f7
to
3165db8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (7)
app/packages/operators/src/useCustomPanelHooks.ts (1)
Line range hint
39-40: Avoid using 'Function' as a type for handlers inCustomPanelHooks.Using 'Function' as a type is unsafe because it accepts any function-like value, which can be a source of bugs. Define the function shape explicitly.
- handlePanelStateChange: Function; - handlePanelStatePathChange: Function; + handlePanelStateChange: (newState: any) => void; + handlePanelStatePathChange: (path: string, value: any, schema: any) => void;app/packages/operators/src/operators.ts (3)
Line range hint
91-91: Correct the order of default parameters in function signatures.Default parameters should follow the last required parameter or be made required to avoid confusion at the call site.
- constructor(public operatorURI: string, public params: unknown = {}, public options?: OperatorExecutorOptions) + constructor(public operatorURI: string, public options?: OperatorExecutorOptions, public params: unknown = {})Also applies to: 155-156, 759-759
Line range hint
213-215: Remove unnecessary boolean literals in conditional expressions.Simplify the code by directly assigning the result without using a ternary operator.
- this.canExecute = options.canExecute === false ? false : true; - this.disableSchemaValidation = options.disableSchemaValidation === true ? true : false; + this.canExecute = options.canExecute !== false; + this.disableSchemaValidation = !!options.disableSchemaValidation;
Line range hint
268-268: Use optional chaining to simplify null checks.Refactor the code to use optional chaining, which makes the code cleaner and more readable.
- if (this.executor && !(this.executor instanceof Executor)) { + if (this.executor?.!(instanceof Executor)) {Also applies to: 560-560, 742-742, 844-844, 893-893
fiftyone/operators/types.py (3)
Line range hint
255-313: Refactor thebtnmethod to improve clarity and maintainability.
- The method
btnhas been updated to include new parameters such ashref. This aligns with the PR objectives to improve button functionalities.- Consider using more descriptive variable names for parameters like
on_clickto clarify that it expects a string representing the function name, not the function itself.- The method could benefit from splitting into smaller methods or using helper functions to handle different types of buttons (e.g., icon button vs. regular button) to improve readability and maintainability.
Line range hint
1132-1141: Remove duplicate dictionary key"href"to prevent potential bugs.- "href": self.href,
- This change addresses the static analysis hint about the repeated dictionary key
"href".
Line range hint
13-13: Remove unnecessary inheritance fromobjectin class definitions.- class BaseType(object): + class BaseType: - class View(object): + class View: - class Placement(object): + class Placement: - class ViewTargetOptions(object): + class ViewTargetOptions:
- This change addresses the static analysis hints about unnecessary inheritance from
object, which is redundant in Python 3.Also applies to: 813-813, 1390-1390, 1859-1859
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (9)
- app/packages/components/src/components/Popout/index.tsx (2 hunks)
- app/packages/core/src/plugins/SchemaIO/components/Button.tsx (1 hunks)
- app/packages/core/src/plugins/SchemaIO/components/ButtonView.tsx (2 hunks)
- app/packages/core/src/plugins/SchemaIO/components/IconButtonView.tsx (4 hunks)
- app/packages/operators/src/operators.ts (1 hunks)
- app/packages/operators/src/useCustomPanelHooks.ts (5 hunks)
- app/packages/operators/src/usePanelEvent.ts (1 hunks)
- app/packages/spaces/src/components/AddPanelButton.tsx (2 hunks)
- fiftyone/operators/types.py (6 hunks)
Files skipped from review due to trivial changes (1)
- app/packages/components/src/components/Popout/index.tsx
Files skipped from review as they are similar to previous changes (5)
- app/packages/core/src/plugins/SchemaIO/components/Button.tsx
- app/packages/core/src/plugins/SchemaIO/components/ButtonView.tsx
- app/packages/core/src/plugins/SchemaIO/components/IconButtonView.tsx
- app/packages/operators/src/usePanelEvent.ts
- app/packages/spaces/src/components/AddPanelButton.tsx
Additional context used
Path-based instructions (2)
app/packages/operators/src/useCustomPanelHooks.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/operators/src/operators.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
Biome
app/packages/operators/src/useCustomPanelHooks.ts
[error] 39-39: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
[error] 40-40: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
app/packages/operators/src/operators.ts
[error] 91-91: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 155-155: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 156-156: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 213-213: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
[error] 215-215: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
[error] 268-268: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 305-305: void is confusing inside a union type. (lint/suspicious/noConfusingVoidType)
Unsafe fix: Use undefined instead.
[error] 313-313: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 560-560: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 742-742: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 759-759: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 844-844: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 893-893: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Ruff
fiftyone/operators/types.py
13-13: Class
BaseTypeinherits fromobject(UP004)Remove
objectinheritance
81-84: Use ternary operator
view = View() if view is None else view.clone()instead ofif-else-block (SIM108)Replace
if-else-block withview = View() if view is None else view.clone()
813-813: Class
Viewinherits fromobject(UP004)Remove
objectinheritance
1390-1390: Class
Placementinherits fromobject(UP004)Remove
objectinheritance
1859-1859: Class
ViewTargetOptionsinherits fromobject(UP004)Remove
objectinheritance
Additional comments not posted (2)
app/packages/operators/src/useCustomPanelHooks.ts (1)
16-26: UpdateCustomPanelPropsinterface to use string types for event handlers.This change aligns with the PR objectives to fix issues with Python panels not loading correctly on app reload, by ensuring that event handlers are consistently represented as strings, which are likely serialized/deserialized across different contexts.
app/packages/operators/src/operators.ts (1)
576-591: RefactorexecuteOperatorto use the updated function signature.The refactoring of
executeOperatorto include an optionaloptionsparameter and the use ofresolveOperatorURIWithMethodto handle the operator URI and parameters is a significant improvement. This change enhances flexibility and clarity in handling operator execution options.
| @@ -54,7 +54,7 @@ function useCtxChangePanelEvent(panelId, value, operator) { | |||
|
|
|||
| export function useCustomPanelHooks(props: CustomPanelProps): CustomPanelHooks { | |||
| const { panelId } = props; | |||
| const [panelState, setPanelState] = usePanelState(null, panelId); | |||
| const [panelState] = usePanelState(null, panelId); | |||
There was a problem hiding this comment.
Ensure proper usage of executeOperator with the new function signature.
The executeOperator function is called multiple times in this file. Given the changes in its signature, ensure that all calls to this function pass the correct parameters as per the new signature. This includes the addition of the options parameter where necessary.
- executeOperator(props.onLoad, { panel_id: panelId, panel_state: panelState?.state });
+ executeOperator(props.onLoad, { panel_id: panelId, panel_state: panelState?.state }, {});Also applies to: 78-85, 113-130, 138-151
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [panelState] = usePanelState(null, panelId); | |
| const [panelState] = usePanelState(null, panelId); | |
| // Assuming the executeOperator function is called somewhere in this file, here is an example update: | |
| executeOperator(props.onLoad, { panel_id: panelId, panel_state: panelState?.state }, {}); |
executeOperatorthird param to be optionshreftobtnutilSummary by CodeRabbit
New Features
popoutPropsin thePopoutcomponent for enhanced customization.variantprop.Enhancements
executeOperatorfunction to support additional options and improved parameter handling.AddPanelButtonwith better styling and scrolling capabilities.ButtonViewandIconButtonViewcomponents with newvariantproperties for more flexible UI design.Bug Fixes
CustomPanelPropsto ensure accurate functionality.