Skip to content
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

[Devtools/GED] Filter in Obbject Inspector #27

Merged
merged 15 commits into from
Jun 11, 2024

Conversation

lexfm
Copy link
Contributor

@lexfm lexfm commented May 29, 2024

Change Type (required)

Implemented filter mode for Object Inspector to be used in the Flow panel for the browser Player Devtools, includes but not limited to:

  • Storybook story implementation.
  • DSL test case.
  • Component test for improved test coverage.
  • Filter component implementation.
OIDEMOPR.mov
  • patch
  • minor
  • major
📦 Published PR as canary version: 0.2.1--canary.27.1370

Try this version out locally by upgrading relevant packages to 0.2.1--canary.27.1370

Version

Published prerelease version: 0.2.1-next.0

Changelog

🐛 Bug Fix

  • [Devtools/GED] Filter in Obbject Inspector #27 (@lexfm)

Authors: 1

Copy link

codecov bot commented May 29, 2024

Codecov Report

Attention: Patch coverage is 96.07843% with 4 lines in your changes missing coverage. Please review.

Project coverage is 47.61%. Comparing base (cbb2eb4) to head (b9b9003).
Report is 8 commits behind head on main.

Files Patch % Lines
...pector/src/components/ObjectInspectorComponent.tsx 96.07% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #27      +/-   ##
==========================================
+ Coverage   43.60%   47.61%   +4.01%     
==========================================
  Files          71       71              
  Lines        1688     1781      +93     
  Branches       28       48      +20     
==========================================
+ Hits          736      848     +112     
+ Misses        951      931      -20     
- Partials        1        2       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lexfm
Copy link
Contributor Author

lexfm commented May 29, 2024

/canary

@lexfm
Copy link
Contributor Author

lexfm commented May 31, 2024

/canary

@lexfm
Copy link
Contributor Author

lexfm commented Jun 5, 2024

/canary

@lexfm lexfm marked this pull request as ready for review June 5, 2024 23:21
@lexfm
Copy link
Contributor Author

lexfm commented Jun 5, 2024

/canary

Copy link
Collaborator

@rafbcampos rafbcampos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice feature! I left a quick change suggestion, but overall it LGTM.

const [filterCriteria, setFilterCriteria] = useState("");
const [resultData, setResultData] = useState(data);

const isObject = (value: any): boolean => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move isObject and getPathvalue from the body of the FilterObjectInspector to the ObjectInspectorComponent.tsx scope (AKA move it to line 7)?
In React, whenever a component re-renders, all the code inside the component's function body runs again.
When we define pure functions (functions that always produce the same output given the same input) inside a component, they are recreated on every render, even though their logic doesn't change.
This can lead to unnecessary memory usage and potentially slower performance, especially for large applications or components that re-render frequently.
Moving pure functions outside the component creates them once when the module loads rather than on every render. This can improve performance by reducing the amount of work React needs to do during re-renders. It also makes our code cleaner and easier to test, as pure functions are generally easier to reason about and isolate in tests than functions tied to a component's lifecycle.

mercillo
mercillo previously approved these changes Jun 10, 2024
@lexfm lexfm merged commit d957944 into main Jun 11, 2024
9 checks passed
@intuit-svc intuit-svc mentioned this pull request Jun 11, 2024
@lexfm lexfm deleted the devtools/GED-filter-object-inspector branch June 11, 2024 19:29
@lexfm lexfm restored the devtools/GED-filter-object-inspector branch June 11, 2024 19:41
@adierkens adierkens deleted the devtools/GED-filter-object-inspector branch June 22, 2024 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants