[WIP]: Fixing filter button in marker hover panel

[KaranPradhan266]

Closes: #5704

Tooltip Filter Button Default Behavior

Changes

• The tooltip filter button now defaults to visible by setting showFilterButton = true in
  src/components/tooltip/Marker.tsx.
• Timeline tooltips explicitly opt out of the filter button by passing
  showFilterButton={false} in:
  • src/components/timeline/Markers.tsx
  • src/components/timeline/TrackNetwork.tsx
  • src/components/timeline/TrackCustomMarkerGraph.tsx
• Marker charts continue to display the filter button via the existing prop in
  src/components/marker-chart/Canvas.tsx.

Tests

• Updated Jest snapshots to reflect the new tooltip behavior and minor download-size text changes:
  • src/test/components/__snapshots__/TrackCustomMarker.test.tsx.snap
  • src/test/components/__snapshots__/TrackNetwork.test.tsx.snap
  • src/test/components/__snapshots__/MenuButtons.test.tsx.snap

[KaranPradhan266]

@canova would love a review when you get a chance. Thanks!

[canova]

Sorry for the late review. It was pretty busy last week and I could only look at it today. And thanks for the PR!

It makes sense to have this prop and set it in these components. But I think the value of showFilterButton === undefined being interpreted as true is a bit unusual. I would prefer to have a readonly hideFilterButton?: boolean; prop, with its default showFilterButton === undefined meaning false. And then we need to invert the boolean values in the components.

Also, I think it would be good to have some an explicit hideFilterButton={true} here:

profiler/src/components/network-chart/NetworkChartRow.tsx

Line 495 in f9525d4

<TooltipMarker

With a comment on top of it, saying something like this: "Network Chart doesn't have sticky tooltips yet. But we should convert it to false once we implement sticky tooltips for the network chart."

Also I think it would make sense to have a test for this. Could you write a simple test inside src/test/components/Marker.test.tsx assigning true and false values and asserting if we have the button or not?

Thanks for the patience and for the contribution!

[canova]

Interesting, the CI wants to revert these changes. I tried it locally and these test also fail locally in my machine. What does it do in your machine when you run yarn test? You might want to update the dependencies with yarn install and do a yarn test -u to update the snapshots.

[canova]

I was a bit puzzled by this default assignment because I didn't see it initially. I don't think we should have the default assignment here.

I think the logic of showFilterButton is a bit unusual, because showFilterButton === undefined means "show", that's why I would prefer to have a hideFilterButton instead. That way we can keep its type readonly hideFilterButton?: boolean; and then we wouldn't need any default assignment here. We can directly check it with hideFilterButton ? ...