Closed Bug 924238 Opened 6 years ago Closed 6 years ago

SideMenuWidget's arrows don't update with black boxing

Categories

(DevTools :: Debugger, defect, P3)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 27

People

(Reporter: fitzgen, Assigned: thomas)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good first bug][mentor=nfitzgerald@mozilla.com][lang=css])

Attachments

(5 files)

The center column with the editor changes to a dark gray but the arrow doesn't. This makes it look weird.
We can either hide the arrow completely, or use a different image. I think we should hide it. It's easier.
Marking good first bug.

We define the CSS that makes the arrows here: http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/devtools/widgets.css#340 (and in browser/themes/{linux, windows})

If the selected source is black boxed, it will match this selector:

  #sources .selected .side-menu-widget-item-checkbox:not([checked])

In the debugger theme css files[0], hide the arrows when the selected source is black boxed.

[0] http://mxr.mozilla.org/mozilla-central/find?string=debugger.css&tree=mozilla-central&hint=themes
Whiteboard: [good first bug][mentor=nfitzgerald@mozilla.com][lang=css]
Hi 

I would like to take this as an introduction to the code

Thomas
Sure thing, check out these pages to learn how to hack on our tools:

  https://wiki.mozilla.org/DevTools/GetInvolved
  https://wiki.mozilla.org/DevTools/Hacking

If you have questions, let me know.
Assignee: nobody → thomas
Thanks Nick!
Here is my first attempt to create a patch
hmm, I not sure where the patch commit message went. Shouldn't it appear in the header of the patch file? I followed the instructions here: 
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Anyway, I think it should be easy to see what's going on in this one.
 
Note: As I do not have a VM at the moment I have not verified this fix on Linux or Windows
Comment on attachment 814924 [details] [diff] [review]
924238-initial.patch

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

Thanks Thomas! This looks good to me. Maybe Benvie can double check windows, and Mihai can double check linux?
Attachment #814924 - Flags: review+
(In reply to Thomas Andersen from comment #5)
> Created attachment 814924 [details] [diff] [review]
> 924238-initial.patch
> 
> Thanks Nick!
> Here is my first attempt to create a patch
> hmm, I not sure where the patch commit message went. Shouldn't it appear in
> the header of the patch file? I followed the instructions here: 
> https://developer.mozilla.org/en-US/docs/
> Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-
> in_for_me.3F

Commits don't really get translated in patches, just the total sum of changes, which is ok since in 99% of cases we push the whole change as a single commit when it goes to the tree.
Looks good on Windows!
Comment on attachment 814924 [details] [diff] [review]
924238-initial.patch

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

::: browser/themes/linux/devtools/debugger.css
@@ +65,5 @@
>  #sources .side-menu-widget-item-checkbox:not([checked]) ~ .side-menu-widget-item-contents > .dbg-breakpoint {
>    display: none;
>  }
>  
> +#sources .selected .side-menu-widget-item-checkbox:not([checked]) ~ .side-menu-widget-item-arrow  {

This is a bit of a costly selector. Can you please use 

#sources .side-menu-widget-item.selected > .side-menu-widget-item-checkbox:not([checked]) ~ .side-menu-widget-item-arrow

instead?
Looks good on Linux!
sure!
This patch should fix this
Is the .side-menu-widget-item-contents.selected used for anything? 

See the attached screenshot
In the debugger's case, yes, it contains the source label string.
Attachment #814924 - Attachment is obsolete: true
Priority: -- → P3
Doesn't apply to fx-team. Please rebase.
Keywords: checkin-needed
Attachment #814924 - Attachment is obsolete: false
https://hg.mozilla.org/integration/fx-team/rev/c792dee47fed
Whiteboard: [good first bug][mentor=nfitzgerald@mozilla.com][lang=css] → [good first bug][mentor=nfitzgerald@mozilla.com][lang=css][fixed-in-fx-team]
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/c792dee47fed
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=nfitzgerald@mozilla.com][lang=css][fixed-in-fx-team] → [good first bug][mentor=nfitzgerald@mozilla.com][lang=css]
Target Milestone: --- → Firefox 27
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.