-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
refactor: Sync Scoping tree with Forms data #12171
Conversation
Codecov Report
@@ Coverage Diff @@
## master #12171 +/- ##
==========================================
+ Coverage 66.42% 66.56% +0.14%
==========================================
Files 1015 1017 +2
Lines 49632 49691 +59
Branches 4839 4856 +17
==========================================
+ Hits 32968 33078 +110
+ Misses 16536 16489 -47
+ Partials 128 124 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Can you summarize what are changes made in this PR? filter scope components are shared by filter_box and native filter component, we need to understand why these changes are necessary. thanks! |
/** Chart state of redux */ | ||
export type Chart = { | ||
id: number; | ||
slice_id: 2107; |
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.
what is 2107
used for?
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.
yes, you are right no need it, updating
We build this Scoping tree component instead old one, it placed in ConfigModal for native filters, it duplicates behaviour of old Scoping tree, but save it in |
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.
Code looks good and tested to correctly populate redux, thanks for this improvement! In the future let's split up refactoring and functional PRs.
SUMMARY
This PR add some functionality to Native Filters Modal config:
☝️ Scoping tree clone logic of old scope tree that worked with
FilterBox
filters. This tree save data in reduxnativeFilters
domain and in future this data will be used to filter specific chartsBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Here example of redux/ui equality after tree items selected:
![image (7)](https://user-images.githubusercontent.com/56388545/105145235-1d55e800-5b07-11eb-877a-5f86ab2c6912.png)
![image (6)](https://user-images.githubusercontent.com/56388545/105145237-1dee7e80-5b07-11eb-8961-6e1494a10414.png)
TEST PLAN
ADDITIONAL INFORMATION