1506540 - Conforming style tweaks to Edit and Resend (button states, header title behaviour)

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P3)

Description

[mcroud]

Attachment: edit-resend-nitpick.png

Three minor nit-picks I have noticed while using the newly designed Edit and Resend panel:

1. (see figure A in the image) In extreme narrow widths, the title in the header wraps. This behaviour seems unique to this panel, all other pane titles overflow off the right edge when there is no more room. 
Additionally, maybe the buttons become left aligned in this rather peculiar width?

2. (see figure B in the image) I had no idea there was such a method as "Options", so this is my error! Increasing the width of this input to make the word "Options" appear comfortably would be great.

3. As we are using Photon Micro buttons can we please also include the correct focus and hover states (colors etc) as written in the design docs here: https://design.firefox.com/photon/components/buttons.html#primary-3  

If a high res mockup would be appreciated for these amends for guidance, let me know and I can cook something up!

Comment 1

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

Thanks for the report Matt!

Honza

Comment 2

[Adam Holm]

I'd be glad to fix these issues!

Comment 3

[Adam Holm]

Attachment: bug1506540.patch

Here's a patch for all the changes, except for the button alignment at small widths of the panel.

@Matt: For the "Cancel" button hover states, in the annotations for the new design you gave me, the button background colors weren't default background colors as per Photon specifications. I gave the buttons Photon's default hover background color values, but I can change them if that's not correct.

I'm having some difficulties with the button alignment. Having them wrap as flexbox items seems to create difficulties aligning them horizontally, and many of my flexbox solutions make the buttons run off the page opposite to how the rest of the panel behaves (running off the page to the right, I can attach a screenshot if that's not a clear description). I'm thinking this is something that might have to be done in JavaScript, e.g. in https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/CustomRequestPanel.js, but I'm not sure. Any suggestions?

Comment 4

[mcroud]

Attachment: buttonstyle.png

Good spot! Yes the Cancel buttons are a yet-to-be-documented button style found in other parts of DevTools (Accessibility panel, Performance, Memory panel).
Look at the attached and use the styling for Dark/Light mode of these existing buttons for Cancel, thanks!

Comment 5

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

Comment on attachment 9027084 [details] [diff] [review]
bug1506540.patch

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

Thanks for working on this!

Looks good to me in general, but maybe we could use an extra @media block for the buttons?

Something like:

@media (max-width: 200px) {
  .network-monitor .custom-request .custom-request-button-container {
    justify-content: flex-start;
  } 
}


... to make sure they are properly left aligned when the side-panel is narrow.

Honza

Comment 6

[Adam Holm]

(In reply to Jan Honza Odvarko [:Honza] (need-info? me) from comment #5)
> Comment on attachment 9027084 [details] [diff] [review]
> bug1506540.patch
> 
> Review of attachment 9027084 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for working on this!
> 
> Looks good to me in general, but maybe we could use an extra @media block
> for the buttons?
> 
> Something like:
> 
> @media (max-width: 200px) {
>   .network-monitor .custom-request .custom-request-button-container {
>     justify-content: flex-start;
>   } 
> }
> 
> 
> ... to make sure they are properly left aligned when the side-panel is
> narrow.
> 
> Honza

Is there a way to make @media rules apply to specific html elements? I was under the impression @media rules only applied to print, screen, and speech. Otherwise, if my browser window is greater than my max-width, the buttons will still become unaligned if I make .custom-request-panel narrow. Or is a narrow browser width the only situation I need to be thinking about?

Comment 7

[Ariel Burone]

Sorry for jumping out of nowhere

> > Something like:
> > 
> > @media (max-width: 200px) {
> >   .network-monitor .custom-request .custom-request-button-container {
> >     justify-content: flex-start;
> >   } 
> > }
> > 
> > 
> > ... to make sure they are properly left aligned when the side-panel is
> > narrow.
> > 
> > Honza
> 
> Is there a way to make @media rules apply to specific html elements? I was
> under the impression @media rules only applied to print, screen, and speech.

Whatever is inside a media query will be applied to the elements you target with a selector.
The query (in this case) is against the viewport width (the devtools window)

> Otherwise, if my browser window is greater than my max-width, the buttons
> will still become unaligned if I make .custom-request-panel narrow. Or is a

This rule would apply only to narrow windows.

> narrow browser width the only situation I need to be thinking about?

@media (max-width: 200px)
Is basically saying: apply whatever is inside me unless the width exceeds 200px

So, for windows from 0 to 200px, the styles will be applied. Otherwise the entire block will be ignored.


@media (max-width: 200px) {           /* for windows smaller than 200px*/
  .custom-request-button-container {  /* apply to .custom-request-button-container */
     justify-content: flex-start;     /* this styles*/
  } 

  /* If you do so, you'll need to add a margin-left to the cancel button inside the mq */

  .custom-request-button-close {     /* target the close button*/  
     margin-left:4px;                /* and give it a margin */
  }
}

Hope this helps,
cheers

Comment 8

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

@Adam, is that clear now?

Honza

Comment 9

[Adam Holm]

Attachment: edit-resend-panel-example.jpg

(In reply to Ariel Burone from comment #7)
> Sorry for jumping out of nowhere

No worries, thank you very much for the explanation :)

(In reply to Jan Honza Odvarko [:Honza] (need-info? me) from comment #8)
> @Adam, is that clear now?
> 
> Honza

Yes; I wasn't realizing that |chrome://devtools/content/netmonitor/index.html| could be targeted specifically with @media rules.

In the attached screenshot is a situation where the Edit and Resend panel can be made narrow enough to reproduce the alignment issue without changing the width of the netmonitor panel and triggering the @media rules. Any suggestions for resolving that?

Comment 10

[Adam Holm]

Attachment: bug1506540.patch

Here's my fixes with the @media rules implemented. 

This still leaves the narrow width situation illustrated by my screenshot above. I've been thinking it could be fixed in one of the react files associated with the Network Details panel (https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/CustomRequestPanel.js, https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/MonitorPanel.js, https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/NetworkDetailsPanel.js), but I'm not as adept with react so I haven't gotten a fix together yet.

Comment 11

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

Comment on attachment 9029549 [details] [diff] [review]
bug1506540.patch

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

Thanks for the patch!

Looks good to me in general (see couple of comments I made), but it would be great to have some feedback from Ariel.

Honza

::: devtools/client/netmonitor/src/assets/styles/CustomRequestPanel.css
@@ +65,5 @@
>  
> +  .network-monitor .custom-request .custom-request-button-container {
> +    display: flex;
> +    flex-direction: row;
> +    flex-wrap: wrap-reverse;

nit: Wrong indentation

::: devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css
@@ +392,5 @@
>  /* Custom request panel */
>  
> +@media (max-width: 130px) {
> +  .network-monitor .custom-request .custom-request-button-container {
> +    justify-content: flex-start;

when using `center` in case the width is smaller than 130px the buttons are nicely aligned.
(but it's a bit surprising that `left` doesn't work that way)

Comment 12

[Ariel Burone]

Hi, sorry for the delay, missed the mail.

Not sure if this is what Honza expected, but I'll try my best :P
I can't apply the patch, I get a strange conflict, so I'm not really sure if I've got the correct stylesheets.
I'll attach 4 images and 2 css files, a patch would probably make things worse.

1st image is what I saw after applying manually Adam's changes. I see a 1px misalignment
2nd misaligned fixed
3rd right aligned buttons (not sure if the intention was getting them left or right aligned)
4th adds vertical space between the buttons when stacked

---

This is a "diff" created manually, sorry for the mess


file: netmonitor/src/assets/styles/CustomRequestPanel.css
.network-monitor .custom-request .custom-request-button-container {
  display: flex;
-  flex-direction:row; /* not necessary, that's the default */
  flex-wrap: wrap-reverse;
}


file: netmonitor/src/assets/styles/NetworkDetailsPanel.css
@media (max-width: 130px) {
  .network-monitor .custom-request .custom-request-button-container {
    justify-content: flex-start;
    padding-left: 16px;
  }

  .network-monitor .custom-request #custom-request-send-button {
-    margin-left: 0; /* fixes the alignment (off by 1px) */
  }
}


---

Not sure if the idea was to have the buttons left or right aligned

This is for right aligned buttons

file: netmonitor/src/assets/styles/NetworkDetailsPanel.css
@media (max-width: 130px) {
 - .network-monitor .custom-request #custom-request-send-button{
 -   margin-left:0;
 - }

  .network-monitor .custom-request .custom-request-button-container{
 -    justify-content:flex-start;
 -    padding-left:16px;
 +    justify-content:flex-end;
 +    margin-right:4px;
  }
}

file: netmonitor/src/assets/styles/CustomRequestPanel.css
@media (min-width: 130px) {
  .network-monitor .custom-request .custom-request-button-container {
    justify-content: flex-end;
 +    margin-right:4px;
  }

 - .network-monitor .custom-request #custom-request-send-button {
 -    margin-left:4px;
 -  }
}

.network-monitor .custom-request #custom-request-send-button {
 -  margin-right:16px;
 +  margin-left:4px;
}

-----
if you want some vertical spacing between the stacked buttons add this block 

@media (max-width: 130px) {
  .network-monitor .custom-request button {
    margin-bottom: 4px;
  }
}

/*
after this block
.network-monitor .custom-request button {}
*/



PS: not related to  this error. But the network panel shouldn't have a horizontal scrollbar when the timeline overflows to the right?
PS2: "Changes" tab can't be used directly as a diff, doesn't give the same contextual lines :.(

Comment 13

[Ariel Burone]

Attachment: 1506540 1 initial

1px misalignment

Comment 14

[Ariel Burone]

Attachment: 1506540 2 fixed margin

Misalignment fixed

Comment 15

[Ariel Burone]

Attachment: 1506540 3 right aligned

Comment 16

[Ariel Burone]

Attachment: 1506540 4 with vertical spacing

Comment 17

[Ariel Burone]

Attachment: CustomRequestPanel.css

Comment 18

[Ariel Burone]

Attachment: NetworkDetailsPanel.css

Comment 19

[Adam Holm]

(In reply to Ariel Burone from comment #12)

Sorry for the delay myself. Thanks for the great feedback!

(In reply to Ariel Burone from comment #14)
> Created attachment 9030142 [details]
> 1506540 2 fixed margin
> 
> Misalignment fixed

Where/how did you fix the misalignment here? I implemented the changes from your CustomRequestPanel.css and NetworkDetailsPanel.css,  other than the changes for right-alignment with the buttons, and I'm still seeing misalignment with the buttons, even without implementing Honza's suggestion to change `justify-content` from `flex-start` to `center` at line 392 in NetworkDetailsPanel.css.


(In reply to Ariel Burone from comment #12)
> Not sure if the idea was to have the buttons left or right aligned

I'm not sure either. I left aligned them based on Matt's mock-up, https://bugzilla.mozilla.org/attachment.cgi?id=9024372.

Comment 20

[Farheen]

Where is the code to fix the bug ? Can anyone help me ?

Comment 21

[u634192]

I would like to contribute to this issue, can someone assign the task to me.

Comment 22

[mariahajmal]

Pls assign it to me, I would like to work on this

Comment 23

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

@Farheen: thanks for helping!

Please read carefully the thread here (there is a lot of useful info).
Especially comment #3, comment #5, comment #7

Related files are:

• https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/CustomRequestPanel.js
• devtools/client/netmonitor/src/assets/styles/CustomRequestPanel.css
• devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css

Honza

Comment 24

[lloan alas:[lloanalas]]

Hi,

I'm an outreachy applicant. It's been a couple of days since last reply, in case this bug becomes available, I'd like to show my interest in working on it. Already read through the threads and understand the issue.

Thanks a ton!

Comment 25

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

@Farheen: are you working on this?
Honza

Comment 26

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

@Sola: still interested in this bug?

Honza

Comment 27

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

NI myself

Comment 28

[u634192]

Yes, I am

Comment 29

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

Assigned to you, thanks!
Honza

Comment 30

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

@Sole: do you need any help?

Honza

Comment 31

[u634192]

I've increased the width of the method input to fit "Options".

Having a bit of trouble getting the title in the edit & resend panel header to overflow instead of wrapping, I have a strong suspicion that it's an overflow/flexbox issue causing the problem. Still digging into it, but I'm open to any suggestions you have.

Haven't started on the buttons yet, but it seems I can mostly apply the changes @Ariel & @Adam made.

Comment 32

[u634192]

Attachment: Bug 1506540 - Conforming style tweaks to Custom Request Panel in net monitor (button states, header title behaviour). r=honza

Increase the minimum width of the custom method value to make all options appear comfortably.
Fix Custom Request Panel header title wrap bug.
Style & position Custom Requeset Panel buttons properly.

Comment 33

[u634192]

Hey Honza!

Just updated the Revision

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e83e4d61954
Conforming style tweaks to Custom Request Panel in net monitor (button states, header title behaviour). r=Honza

Comment 35

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/3e83e4d61954