1485416 - Highlight tracker in the Headers side panel

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P3)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

This is a follow up for bug 1333994.

The headers panel should render a note about tracker resources.

See this mockup:
https://bugzilla.mozilla.org/attachment.cgi?id=9003071&action=edit

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

See also:
https://docs.google.com/document/d/1Lu7XRijRyIp6wuwlIx72kIwPi77RlkEUkdTglaARgrU/edit#heading=h.8co0k7d1ovg7

Honza

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: tracker-sidepanel.jpg

Attaching a mockup of the required UI.
This is the version we agreed on.
Honza

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Some instructions for anyone interested in this bug:

1) Here is the file/component that represents the Headers side panel
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/HeadersPanel.js

2) Here is usage of the property that can be used to see whether a resource is tracker or not:
https://searchfox.org/mozilla-central/rev/dd965445ec47fbf3cee566eff93b301666bda0e1/devtools/client/netmonitor/src/components/RequestListColumnDomain.js#58

3) The Headers panel needs to check that prop and display the additional message (see the mockup for design)

Honza

Comment 4

[Joshua]

It would not highlight and I was able to reproduce the error. Could I be assigned to this bug and get access to the docs?

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Done!

Honza

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Joshua: any progress on this bug? Can I help you somehow?
Honza

Comment 7

[catherinelesnova]

Hello! 
I need some time to make sense, but I want to try.
Can I access the documents?

Comment 8

[Jiachen Jiang]

Yes, I would be interested!  Could I get assigned to the bug?

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to catherinelesnova from comment #7)
> Hello! 
> I need some time to make sense, but I want to try.
> Can I access the documents?
Hi!
not sure what documents do you have in mind...?
But if you are interested in fixing a bug and you are looking for first-good-bug, please look at this list: https://mzl.la/2zCcSFE You can pick a bug and post a comment saying that you want to be assigned to it.

Honza

Comment 10

[Hengyeow (:tanhengyeow)]

Hi Jiachen

Are you still working on this? Do you require any assistance? :)

Comment 11

[Hengyeow (:tanhengyeow)]

Attachment: Bug 1485416 - Highlight tracker in the Headers side panel. r=Honza

Include highlight tracker in the Headers side panel

Comment 12

[Hengyeow (:tanhengyeow)]

Attachment: theme-dark-tracking-protection.png

Comment 13

[Hengyeow (:tanhengyeow)]

Attachment: theme-light-tracking-protection.png

Comment 14

[Hengyeow (:tanhengyeow)]

Points to note:

• Although the mockup places a Learn more hyperlink for users to find out more about Tracking Protection, I used the ? icon instead to ensure consistency between all other MDN helper links in the Headers panel.
• How it looks on both light and dark themes are shown in the attached images.

Comment 15

[Hengyeow (:tanhengyeow)]

Hi Honza

Also I have an interesting question that popped up in my mind while cracking on this bug. What is the ideal workflow for the netmonitor to pass data between components? Not necessarily between parent/child components but components living far apart from each other). I know there are 2 ways currently:

1. Pass the value to the root parent component that manages both components that are affected using callbacks/states, then pass the value down to the desired child component
2. Using react/redux

I guess the second way is preferred and I've seen some react/redux function calls in the codebase. But is there a more concrete example in the codebase I can refer to for this type of interaction?

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: image.png

Thanks for working on this!

See my attached screenshot. It would be great if the info (tracking icon + description + learn more icon) would be rendered on the same line -> to save space when the side bar is rather thinner.

Honza

Comment 17

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Heng Yeow (:tanhengyeow) from comment #15)

I guess the second way is preferred and I've seen some react/redux function calls in the codebase.

Yes

But is there a more concrete example in the codebase I can refer to for this type of interaction?

We are usually using Redux's connect method:
see e.g. here:
https://searchfox.org/mozilla-central/rev/dc0adc07db3df9431a0876156f50c65d580010cb/devtools/client/netmonitor/src/components/NetworkDetailsPanel.js#76-86

module.exports = connect(
  (state) => ({
    activeTabId: state.ui.detailsPanelSelectedTab,
    request: getSelectedRequest(state),
  }),
  (dispatch) => ({
    cloneSelectedRequest: () => dispatch(Actions.cloneSelectedRequest()),
    selectTab: (tabId) => dispatch(Actions.selectDetailsPanelTab(tabId)),
    toggleNetworkDetails: () => dispatch(Actions.toggleNetworkDetails()),
  }),
)(NetworkDetailsPanel);

This way you can access any data in the store/reducer and pass them as props in the component
You can also use selectors (e.g. getSelectedRequest from the example above), which might pre-proccess data and perhaps also memoize them.

We could also use 'react context', see:

• https://reactjs.org/docs/context.html
• https://hackernoon.com/how-do-i-use-react-context-3eeb879169a2

It isn't utilized by Net panel yet, but I wouldn't be against that - for cases where Redux state is not applicable.

Honza

Comment 18

[Hengyeow (:tanhengyeow)]

Hi Honza

Thanks for the detailed explanation!

I've updated the patch to not use flexbox, the reason why it behaved the way you saw in the screenshot. The desired behaviour can be seen now.

However, the items are slightly misaligned now because it can't be nicely aligned in the center using flexbox. I tried a few variations but the best way to keep the desired behaviour is to not use flexbox:
It would be great if the info (tracking icon + description + learn more icon) would be rendered on the same line -> to save space when the side bar is rather thinner.

Comment 19

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the update!

Some comments:

1. Can we apply the following CSS props

.tracking-protection
// To increase spacing from other labels (I think the mockup is also using some spacing)
margin-top: 2px;
margin-bottom: 2px;

// .tracking-resource
// To improve the v-alignment a bit
vertical-align: middle;

2. The tracking-resource icon should be white in both dark/light mode (just like if it's selected in the list of resources)

Honza

Comment 20

[Hengyeow (:tanhengyeow)]

Hi Honza

Updated the patch! White doesn't go well with grey background so I used black instead (same as what mockup suggested). But I'm using white for dark mode.

Try results here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a9cb00c43c40c82e7d5362949ae838ff0baf5b6

Comment 21

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Looks great, thanks!

Landed

Honza

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c68cc2526fbd
Highlight tracker in the Headers side panel. r=Honza

Comment 23

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/c68cc2526fbd

Comment 24

[Irene Smith]

On the headers section of the Network Request details page, I added the following information along with a screenshot:

Starting in Firefox 67, in addition to showing information about known trackers in the list, the request information section of the Headers panel also shows an icon and a message if the request is to a site that is associated with a known tracker ({{bug("1485416")}}).

Comment 25

[Hengyeow (:tanhengyeow)]

Hi Irene

Thanks for the update! The changes look good to me :)