-
Notifications
You must be signed in to change notification settings - Fork 144
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
✨ [RUM-4908] Add only
Link to Facet List in Developer Extension
#2830
Conversation
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
developer-extension/src/panel/components/tabs/eventsTab/facetList.tsx
Outdated
Show resolved
Hide resolved
developer-extension/src/panel/components/tabs/eventsTab/facetList.tsx
Outdated
Show resolved
Hide resolved
c7f7816
to
14e36b9
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2830 +/- ##
==========================================
- Coverage 93.50% 92.95% -0.55%
==========================================
Files 290 297 +7
Lines 7703 7840 +137
Branches 1754 1785 +31
==========================================
+ Hits 7203 7288 +85
- Misses 500 552 +52 ☔ View full report in Codecov by Sentry. |
}) { | ||
const isTopLevel = depth === 0 | ||
const isSelected = !excludedFacetValues[facet.path] || !excludedFacetValues[facet.path].includes(facetValue) | ||
const facetSelectState = computeSelectionState(facetValuesFilter, facetRegistry, facet, facetValue, parentList) | ||
const isCollapsed = |
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.
Not directly related to the only
feature but I would decorrelate collapse state from the filtering state and add icons to control collapsing manually.
For example, if I want to only select css
and image
I would start by unselecting resource
and then clicking on both. But right now it will unselect and collapse resource
.
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.
🤔 I actually do the opposite, I never unselect the parent first. Right now, I aim to mimick the web-ui facets, when selecting a child, we expand the parent and find the child to select. But I agree the collapsing is limited here as we are using a third party. Maybe we implement our own collapse component separately, because I worry this PR is growing a bit much.
developer-extension/src/panel/components/tabs/eventsTab/computeFacetState.ts
Show resolved
Hide resolved
developer-extension/src/panel/components/tabs/eventsTab/computeFacetState.ts
Outdated
Show resolved
Hide resolved
e613fc6
to
432e694
Compare
) | ||
) | ||
) | ||
const filteredEvents: SdkEvent[] = [] |
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.
💬 suggestion: I would keep the filter
approach which is maybe more clear.
const filteredEvents: SdkEvent[] = [] | |
export function filterFacets( | |
events: SdkEvent[], | |
facetValuesFilter: FacetValuesFilter, | |
facetRegistry: FacetRegistry | |
): SdkEvent[] { | |
const filteredFacetValueEntries = Object.entries(facetValuesFilter.facetValues) | |
if (filteredFacetValueEntries.length === 0) { | |
return events | |
} | |
const isIncludeType = facetValuesFilter.type === 'include' | |
return events.filter((event) => | |
filteredFacetValueEntries.some(([facetPath, filteredValues]) => { | |
const eventValue = facetRegistry.getFieldValueForEvent(event, facetPath) | |
return isIncludeType == filteredValues.includes(eventValue as 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.
Yes, this is way more clear!
3ca0ace
to
5c1218c
Compare
--wip-- [skip ci] --wip-- [skip ci] --wip-- [skip ci]
--wip-- [skip ci]
5c1218c
to
498fead
Compare
Notes: Some commits were not signed before multiple times of merging from main, hence rebase and cherry-pick were necessary. |
Motivation
Add
only
link to facet list in developer extension, so we don't need to click on every element to exclude everything.Changes
Screen.Recording.2024-08-06.at.11.12.43.mov
Testing
I have gone over the contributing documentation.