1328313 - Caret icons should be visible in request-menu-toolbar [devtools>network tab]

Status: RESOLVED WONTFIX

(DevTools :: Netmonitor, defect, P3)

Description

[[:Towkir] Ahmed]

Attachment: devtools_networktab_tableheader.gif

STR:
1. Open Developer Tools
2. Go to network tab
3. Hover on the table header [..Status  |  Method  |  File  |  Domain  | Cause  ..]
4. Now click on any of them, and you will see a caret icon right to the menu that explains that these values can be sorted by clicking on them.

Result:
At step 3 when you hover on them, they change their background color a little bit, but no caret icon is visible on mouse hover.

Expectation:
It is necessary for a new user to understand that these values can be sorted by clicking on the menus, and this would be nice if the caret icon [button-icon in codebase] became visible at mouse over (at step 3)

See the attached .gif to understand the issue.

Comment 1

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

Yep, agree. Thanks for the report!

Honza

Comment 2

[[:Towkir] Ahmed]

There is another thing to mention, 
Currently, whenever the requests load for the first time they are not sorted ascending or descending (I am not sure if they are sorted or not, if sorted, don't know the order)
So,
What should be the icon on hover if they are sorted in none of the ascending or descending order ? As we have only two possible caret, up and down.

Comment 3

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

The list of requests is sorted by time by default and the 'Timeline' column should indicate that automatically by displaying the ascending icon.

Honza

Comment 4

[[:Towkir] Ahmed]

Alright,
If this is an easy or intermediate one, I can take this if you are eager to mentor :)

Comment 5

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

(In reply to [:Towkir] Ahmed from comment #4)
> Alright,
> If this is an easy or intermediate one, I can take this if you are eager to
> mentor :)
Yes, please :-)

Honza

Comment 6

[[:Towkir] Ahmed]

Assigning for now, I will let you know about my progress :)

Comment 7

[[:Towkir] Ahmed]

Hi Honza,
As far as I could understand, the styles for the caret icon we are talking about here is styled from here:
https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/netmonitor.css#233
I can see there is no background image set unless the .requests-menu-header-button element has an attribute of [data-sorted=descending] or [data-sorted=ascending]
Can you help me here on how to implement this ? Do we need JS for this or CSS is enough ? Or am I not even close to it ?

Thanks

Comment 8

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

(In reply to [:Towkir] Ahmed from comment #7)
> Hi Honza,
> As far as I could understand, the styles for the caret icon we are talking
> about here is styled from here:
> https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/
> netmonitor.css#233
Correct

> I can see there is no background image set unless the
> .requests-menu-header-button element has an attribute of
> [data-sorted=descending] or [data-sorted=ascending]
Yes

> Can you help me here on how to implement this ? Do we need JS for this or
> CSS is enough ? Or am I not even close to it ?
The markup: <div class="button-icon"></div> is there all the time (with correct size) so, we need to introduce a new CSS style that sets the background on mouse hover:

Something like as follows:

/* Display the default ascending icon to indicate sorting feature */
.requests-menu-header-button:hover > .button-icon {
  background-image: var(--sort-ascending-image);
}

1) We should also use 'filter' CSS prop to gray out the icon a bit.
2) It should probably appear only if the hovered column isn't sorted already.

Does that make sense?

Honza

Comment 9

[[:Towkir] Ahmed]

(In reply to Jan Honza Odvarko [:Honza] from comment #8)
> /* Display the default ascending icon to indicate sorting feature */
> .requests-menu-header-button:hover > .button-icon {
>   background-image: var(--sort-ascending-image);
Yes
> 2) It should probably appear only if the hovered column isn't sorted already.
I am planning for something like this:
.requests-menu-header-button:not([data-sorted]):hover > .button-icon {
  background-image: var(--sort-ascending-image);
  ........
}
> 1) We should also use 'filter' CSS prop to gray out the icon a bit.
I think I should add the filter property on the dotted place up there. 
How much do you want the value to be ?

Thanks

Comment 10

[[:Towkir] Ahmed]

filter: grayscale(value);  doesn't really make any changes to the visibility of this.
how about an opacity: 0.3;  ??

Comment 11

[[:Towkir] Ahmed]

Beside, do we have any icon both up and down together ? 
Something like this ?
    
    /\
    \/

--------------------
Thanks

Comment 12

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

(In reply to [:Towkir] Ahmed from comment #10)
> filter: grayscale(value);  doesn't really make any changes to the visibility
> of this.
> how about an opacity: 0.3;  ??
It's ok to use opacity


(In reply to [:Towkir] Ahmed from comment #11)
> Beside, do we have any icon both up and down together ? 
> Something like this ?
I don't think so but, why do you need it?
Note that every column is sorted by default so, we can display the right icon immediately, no?

Honza

Comment 13

[[:Towkir] Ahmed]

Attachment: caret_up_or_up_down.gif

(In reply to Jan Honza Odvarko [:Honza] from comment #12)
> Note that every column is sorted by default so, we can display the right
> icon immediately, no?
Notice the file column, when I hover, it shows the up caret, but the file column is not definitely sorted the way it sorts when I click it first time,
Is it logical to show the up caret on hover and also after the click when it sorts for the first time ?

I thought it would be better to use a neutral icon for sorting for the first time only, after first click, all are good as they way the are currently.

That's why I asked about this:
---------------------
        /\
        \/
---------------------

Still I can submit a patch if you are strict on this about the up caret icon.

I am adding the suggested rule at this line:
https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/netmonitor.css#240

Comment 14

[[:Towkir] Ahmed]

Attachment: the_way_bugzilla_sorts.gif

Hi Honza, meanwhile I found the best example to explain what I was trying to do here.
See how bugzilla dashboard sorts the items and the icons on the header for the first time when they are untouched. :)

Comment 15

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

OK, I see what you mean and yes it make sense. I think that you might try to search Firefox source to get the icon of use the one Bugzilla is using, I guess.

Honza

Comment 16

[[:Towkir] Ahmed]

Attachment: proposing_sort_icon.png

I am not sure if this is a fix or a workaround, but I could manage to create a sort icon by this rule, What do you guys think ? the attachment shows the result on windows, HiDPI monitor, will check what happens on linux by tonight, meanwhile you can suggest improvements on the css rule.

.requests-menu-header-button:not([data-sorted]):hover > .button-icon {
	height: 10px;
	background-image: var(--sort-ascending-image), var(--sort-descending-image);
	background-position: 0 0px, 0 6px;
	background-repeat: no-repeat, no-repeat;
}

----------------------------------------
Note: the attached result is on windows.

Comment 17

[[:Towkir] Ahmed]

Re-writing the rule

.requests-menu-header-button:not([data-sorted]):hover > .button-icon {
	height: 10px;
	background-image: var(--sort-ascending-image), var(--sort-descending-image);
	background-position: 0 0, 0 6px;
	background-repeat: no-repeat, no-repeat;
        opacity: 0.3;
}

Comment 18

[Tim Nguyen :ntim]

(In reply to [:Towkir] Ahmed from comment #16)
> Created attachment 8830142 [details]
> proposing_sort_icon.png
> 
> I am not sure if this is a fix or a workaround, but I could manage to create
> a sort icon by this rule, What do you guys think ? the attachment shows the
> result on windows, HiDPI monitor, will check what happens on linux by
> tonight, meanwhile you can suggest improvements on the css rule.
> 
> .requests-menu-header-button:not([data-sorted]):hover > .button-icon {
> 	height: 10px;
> 	background-image: var(--sort-ascending-image), var(--sort-descending-image);
> 	background-position: 0 0px, 0 6px;
> 	background-repeat: no-repeat, no-repeat;
> }
> 
> ----------------------------------------
> Note: the attached result is on windows.

I don't think the double icon makes sense. Bugzilla uses it for the unsorted case, which doesn't make much sense, an arrow represents sorting, so by that logic unsorted should use no icon.

Here's what I believe we should have:

- No icon if no sorting is set
- Up icon for ascending order
- Down icon for descending order

So by default, the timeline header should be selected with an up icon (it's not the case currently, but we should fix that).

As for the icon appearing on hover, I'm less sure about this. If we want to show to the user what's about to happen next, we should also show a down arrow on hover when the user hovers a header with an up arrow, and I find that more confusing than anything.

I do think it would really help selecting the timeline header by default in this case.

Last thing, whatever we end up doing, it should end up consistent with the table headers in the Firefox preferences, and the storage inspector.

Comment 19

[[:Towkir] Ahmed]

(In reply to Tim Nguyen :ntim from comment #18)
> I don't think the double icon makes sense. Bugzilla uses it for the unsorted
> case, which doesn't make much sense, 
I am not sure why you think that.
> an arrow represents sorting, so by that logic unsorted should use no icon.
An arrow up means sort ascending, or down for sorting descending, no arrow(on hover) may mean there is no sorting option available. Here I wanted to make some icons visible on hover so that it becomes clear for a new user that they can sort the columns.

> So by default, the timeline header should be selected with an up icon (it's
> not the case currently, but we should fix that).
That is what Honza suggested
> As for the icon appearing on hover, I'm less sure about this. If we want to
> show to the user what's about to happen next, we should also show a down
> arrow on hover when the user hovers a header with an up arrow, and I find
> that more confusing than anything.
While hovering on something, user would like to know what is gonna happen next. (some tooltip or instructive contents) 
The up+down arrow would be visible on a header only if the header is not already sorted (asc or desc) after the user clicks a header everything should remain as they are now.

Comment 20

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

While I like the double up+down icon, Time is Tim is making good points. I took a look at Chrome tools (and also Google docs/sheets and other Firefox panels) - the double icon isn't used - except of Bugzilla Dashboard. 

So, I suggest to use this bug and fix the Timeline column that should be sorted by default.

In the meantime I'll ask around what folks think about having the additional double icons.

@Towkir: does that work for you?


Thanks!
Honza

Comment 21

[[:Towkir] Ahmed]

Attachment: networktab-caret-icon.patch

Alright, got it.
here is the patch according to our discussion before comment 11

Honza, I would like to file a bug for the same issue in Storage columns if you allow me to.

Hope this patch helps :)
Thanks

Comment 22

[Tim Nguyen :ntim]

Comment on attachment 8830338 [details] [diff] [review]
networktab-caret-icon.patch

Review of attachment 8830338 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/themes/netmonitor.css
@@ +250,5 @@
>  }
> +.requests-menu-header-button:not([data-sorted]):hover > .button-icon {
> +  background-image: var(--sort-ascending-image);
> +  opacity: 0.3;
> +}

comment 18 still applies here.

In your proposal, we only show what's going to happen next on hover in one specific case: the case where no sorting is set yet. I find that very inconsistent.

If we follow this logic, for consistency, the ascending arrow should be replaced by the descending arrow on hover. Similarly, the descending arrow should become the ascending arrow on hover. In this case, seeing arrows twitch on hover would really be confusing.


Not to mention this convention isn't followed anywhere else in any platform/app (about:preferences, bookmarks manager in Firefox, Windows/OSX file explorer, ...).

Comment 23

[Tim Nguyen :ntim]

(In reply to [:Towkir] Ahmed from comment #19)
> (In reply to Tim Nguyen :ntim from comment #18)
> > I don't think the double icon makes sense. Bugzilla uses it for the unsorted
> > case, which doesn't make much sense, 
> I am not sure why you think that.
> > an arrow represents sorting, so by that logic unsorted should use no icon.
> An arrow up means sort ascending, or down for sorting descending, no
> arrow(on hover) may mean there is no sorting option available. Here I wanted
> to make some icons visible on hover so that it becomes clear for a new user
> that they can sort the columns.
If this is to be less confusing for the user, I think we should simply use the same convention as other apps/platforms. Users will likely find what they're used to less confusing.

> > As for the icon appearing on hover, I'm less sure about this. If we want to
> > show to the user what's about to happen next, we should also show a down
> > arrow on hover when the user hovers a header with an up arrow, and I find
> > that more confusing than anything.
> While hovering on something, user would like to know what is gonna happen
> next. (some tooltip or instructive contents) 
A tooltip would be a better way of instructing that.

Comment 24

[[:Towkir] Ahmed]

I agree with you Tim,
So, finally what should we do here ? what is your suggestion, so that I can submit a patch that solves this. :)
I submitted the previous one according to Honza's comment 3 and comment 8
Anyway, I am asking this again because comment 23 is not 'totally' clear to me. I get it that we need tooltip, what about the icon ? 
Thanks
BTW, let me know what you plan for the storage columns

Comment 25

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

Here is what I think we should do:

- The Timeline column should be sorted by default.
- No icon if a column is not sorted.

Honza

Comment 26

[[:Towkir] Ahmed]

Got it,
Going to be WONTFIX then :)

Comment 27

[[:Towkir] Ahmed]

Comment on attachment 8830338 [details] [diff] [review]
networktab-caret-icon.patch

Clearing the review