Skip to content

/ debugger

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

Jump to bottom

[XHR Breakpoints] Make the method selector smaller #8141

Open
fvsch opened this issue Mar 19, 2019 · 19 comments
Open

[XHR Breakpoints] Make the method selector smaller #8141

fvsch opened this issue Mar 19, 2019 · 19 comments
Assignees
@kushthedude
Labels
👋 good first issue in progress

Comments

@fvsch
Copy link
Member

@fvsch fvsch commented Mar 19, 2019

With @jaril's work in #8094 and firefox-devtools/ux#45 we are going to have smaller XHR breakpoint items, 20px tall.

When going to edit mode, though, the input takes more vertical space:

  1. The container's 1px border makes the height 22px instead of 20px. Maybe we should try an inset outline instead? As in: outline: solid 1px; outline-offset: -1px;.
  2. The <select> itself is rather big, at least on Linux its easily more than 30px tall.

We have a minimal <select> style used in Firefox DevTools, for instance in the Responsive Design Mode toolbar or in the Network panel toolbar. We should reuse that style here.

I'm getting good results with this CSS:

.xhr-input-method {
  margin-inline-start: 4px;
  padding-block: 0;
  padding-inline-start: 2px;
  padding-inline-end: 14px;
  border: none;
  border-radius: 2px;
  font: inherit;
  color: inherit;
  background-color: var(--toolbarbutton-background);
  -moz-appearance: none;
  appearance: none;
}

Then the only thing missing is adding the double-arrow icon to the right of the select.
@claim claim bot added the not-available label Mar 19, 2019
@fvsch fvsch mentioned this issue Mar 19, 2019
Merged
@fvsch
Copy link
Member Author

@fvsch fvsch commented Mar 21, 2019

There's a UI spec draft in firefox-devtools/ux#50
But I see the value in being consistent with the select buttons in RDM / Network, so I would still go with that.
@fvsch fvsch mentioned this issue Mar 26, 2019
Closed
@blueberryapple
Copy link

@blueberryapple blueberryapple commented Apr 24, 2019

/claim
@claim claim bot added the in progress label Apr 24, 2019
@claim
Copy link

@claim claim bot commented Apr 24, 2019

Thanks for claiming the issue! 👋

!!! Please check your email and confirm the invitation

Here are some links for getting setup, contributing, and developing. We're always happy to answer questions in slack! If you become busy, feel free to /unclaim it.

🦊 Debugger team!
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 3, 2019
@blueberryapple
got the arrow to show up with the style recommended in the issue (fir…
9816401 
…efox-devtools#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 3, 2019
@blueberryapple
got the arrow to show up with the style recommended in the issue (fir…
9d51e45 
…efox-devtools#8141)
@blueberryapple
Copy link

@blueberryapple blueberryapple commented May 3, 2019 •
edited

so I added the css changes suggested, this is what it currenly looks like after adding in the background image for the arrow:

image

I'm not too sure how to transform the svg color, since the other places use background-color to change the icon which wouldn't really work in this scenario.
@fvsch
Copy link
Member Author

@fvsch fvsch commented May 3, 2019 •
edited

Hmm. We have a way that works in Firefox only, using fill="context-fill" in the SVG code and this in the CSS:

-moz-context-properties: fill;
fill: var(--theme-icon-dimmed-color);

It won't work in Chrome but maybe that's an acceptable trade-off?
Especially if we use a medium gray fallback that is still visible in Chrome, e.g.:

<path fill="context-fill #808080" d="..." />
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 4, 2019
@blueberryapple
changed fill color and merged background css properties (firefox-devt…
8a62f60 
…ools#8141)
@blueberryapple
Copy link

@blueberryapple blueberryapple commented May 4, 2019 •
edited

looks like that works when it's an svg element, but not when the svg is used as a background image 😢 . the fallback value is parsed in firefox, but not in chrome- so we end up with a gray arrow in firefox:

image.

and a black arrow in chrome:

image.

so I just changed the fill color to #808080 and both browsers now have gray arrows as expected, but this isn't the optimal solution is it?
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 8, 2019
@blueberryapple
using pseudo-elements in order to not modify svg and be inline with h…
388170f 
…ow we are using mask to set the color for the arrrow.svg (firefox-devtools#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 8, 2019
@blueberryapple
cleaned up the redundant styles for the input container pseudoelement (…
b7569b1 
…firefox-devtools#8141)
@blueberryapple
Copy link

@blueberryapple blueberryapple commented May 8, 2019 •
edited

Okay! I got this to work great without needing to modify the svg with pseudoelements 😄. With the pseudoelement, we can use mask and set the svg colors like the other places no problem. @fvsch What do you think?
@fvsch
Copy link
Member Author

@fvsch fvsch commented May 8, 2019

Given that rendering in Firefox devtools is our primary use case and supports -moz-context-properties: fill; and fill="context-fill", I don't know if we need to make the CSS and DOM structure more complex to use a mask here. :/ But if the fallback gray color does not work in Chrome at all, I guess the pseudo-element technique might be better.

Two things on the design side:

  • the icon we want to use is the same as the one used in Network (Throttling, HAR), aka chrome://devtools/skin/images/select-arrow.svg;
  • (small correction, sorry) we're using --theme-icon-color and not --theme-icon-dimmed-color for this icon.
@fvsch
Copy link
Member Author

@fvsch fvsch commented May 8, 2019

@blueberryapple Can you make a pull request, to continue the discussion on the code?
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 9, 2019
@blueberryapple
addressed fvsch's comments (firefox-devtools#8141)
c3cd98e
@blueberryapple blueberryapple mentioned this issue May 9, 2019
Closed
0 of 4 tasks complete
@blueberryapple
Copy link

@blueberryapple blueberryapple commented May 9, 2019

PR created!
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 9, 2019
@blueberryapple
added double colons for pseudoelement (firefox-devtools#8141)
fcfd105
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 13, 2019
@blueberryapple
changed which element the pseudoelement belonged to (firefox-devtools…
8deccd5 
…#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 13, 2019
@blueberryapple
accounting for rtl and ltr layouts (firefox-devtools#8141)
f566077
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 14, 2019
@blueberryapple
updated snapshot for xhrbreakpoints (firefox-devtools#8141)
a20d96b
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 14, 2019
@blueberryapple
put in fix for failing jest unit test, thanks fvsch! (firefox-devtool…
e525a19 
…s#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
got the arrow to show up with the style recommended in the issue (fir…
fefa509 
…efox-devtools#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
changed fill color and merged background css properties (firefox-devt…
4f24fac 
…ools#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
using pseudo-elements in order to not modify svg and be inline with h…
0d19d78 
…ow we are using mask to set the color for the arrrow.svg (firefox-devtools#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
cleaned up the redundant styles for the input container pseudoelement (…
0e828c8 
…firefox-devtools#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
addressed fvsch's comments (firefox-devtools#8141)
973e5c4
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
added double colons for pseudoelement (firefox-devtools#8141)
9b98267
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
changed which element the pseudoelement belonged to (firefox-devtools…
ce426db 
…#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
accounting for rtl and ltr layouts (firefox-devtools#8141)
f08ffe6
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
updated snapshot for xhrbreakpoints (firefox-devtools#8141)
b7caa0f
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
put in fix for failing jest unit test, thanks fvsch! (firefox-devtool…
e8e3266 
…s#8141)
blueberryapple added a commit to blueberryapple/debugger that referenced this issue May 21, 2019
@blueberryapple
updated snapshots (firefox-devtools#8141)
b7a3bc6
@Jasbir96
Copy link

@Jasbir96 Jasbir96 commented Aug 13, 2019

/claim
@claim
Copy link

@claim claim bot commented Aug 13, 2019

Thanks for claiming the issue! 👋

!!! Please check your email and confirm the invitation

Here are some links for getting setup, contributing, and developing. We're always happy to answer questions in slack! If you become busy, feel free to /unclaim it.

🦊 Debugger team!
@vishakhanihore
Copy link

@vishakhanihore vishakhanihore commented Nov 16, 2019

/claim
@claim
Copy link

@claim claim bot commented Nov 16, 2019

Thanks for claiming the issue! 👋

!!! Please check your email and confirm the invitation

Here are some links for getting setup, contributing, and developing. We're always happy to answer questions in slack! If you become busy, feel free to /unclaim it.

🦊 Debugger team!
@vishakhanihore
Copy link

@vishakhanihore vishakhanihore commented Nov 16, 2019

/unclaim
@kushthedude
Copy link

@kushthedude kushthedude commented Nov 16, 2019

Hello @fvsch , I would like to work on this issue .
@claim
Copy link

@claim claim bot commented Nov 16, 2019

Thanks for claiming the issue! 👋

!!! Please check your email and confirm the invitation

Here are some links for getting setup, contributing, and developing. We're always happy to answer questions in slack! If you become busy, feel free to /unclaim it.

🦊 Debugger team!
@kushthedude
Copy link

@kushthedude kushthedude commented Nov 16, 2019

/claim
@claim
Copy link

@claim claim bot commented Nov 16, 2019

Thanks for claiming the issue! 👋

Here are some links for getting setup, contributing, and developing. We're always happy to answer questions in slack! If you become busy, feel free to /unclaim it.

🦊 Debugger team!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kushthedude kushthedude

Labels
👋 good first issue in progress
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Feature/8141: XHR Breakpoint Styling Changes
7 participants
@darkwing @fvsch @AnshulMalik @blueberryapple @Jasbir96 @kushthedude @vishakhanihore