1493749 - RDM toolbar icons, sizes and colors diverge from toolbox

Status: NEW

(DevTools :: Responsive Design Mode, enhancement, P3)

Description

[:Harald Kirschner :digitarald]

Different icons/sizes:

- Camera
- Settings cog

Different colors: Toolbox uses --theme-toolbar-photon-icon-color while RDM uses --theme-toolbar-color

Related, the RDM settings menu doesn't use the dropdown design with pointy arrow.

Comment 1

[:Harald Kirschner :digitarald]

Attachment: rdm.png

Comment 2

[Megan Bailey]

I can work on this issue. The icon color should be updated, but I'm not sure the arrows are something that should be added. The RDM menu doesn't have submenu items to expand - just the device mode to select. So the checkmark makes sense there. If that needs to be changed as well, let me know.

Comment 3

[Patrick Brosset <:pbro>]

Let me ping Victoria for some guidance here. Victoria, what do you think about this?

Comment 4

[Victoria Wang [:victoria]]

Sorry for the late response to this!

We've had a mix of icons that are different colors to help with the difference in icon weight - Gray 90 for old 1px-style icons, and Gray 90 at 80% for new 2px-style photon icons. There's a bug in the works to update the remaining RDM icons. We can revisit after that lands.

The RDM dropdown seems like it can stay native for now, since it doesn't need icons (yet) which are what require the custom doorhanger.

Comment 5

[Patrick Brosset <:pbro>]

Hi Megan, are you still interested in working on this bug? If not, that's fine, let me know so I can make it available for others to pick up.

Comment 6

[Megan Bailey]

I reached out to Micah with some issues, but I haven't gotten a response. Possibly, you might be able to help me?

As a preface:
1 - I use VSCode, which integrates in to Hg and usually perform commits using their interface, rather than CLI.
2 - I am definitely more used to Git commits, where each commit stands separate.
3 - The first bug I worked on is closed, but when I try to run moz-phab, it still pulls up this commit. Because of this, I have to specify the commit that I want to push for review
3 - I made the corrections to the placeholder highlight bug. I performed a commit. But when I try to submit for review with moz-phab, I run into a problem. The commit is listed under another commit for the other bug I'm working on.
I am not sure how to separate the commits or if I should fix the other bug as well and push for review or what I should do. I have the fixes done.

Comment 7

[Patrick Brosset <:pbro>]

(In reply to Megan Bailey from comment #6)

I reached out to Micah with some issues, but I haven't gotten a response. Possibly, you might be able to help me?

I'll try :)

2 - I am definitely more used to Git commits, where each commit stands separate.

You should feel free to create as many commits as you think you need. You can push multiple separate commits to phabricator. I personally use moz-phab to push and I often do things like moz-phab submit .^ . (which, in this case means: push a range of commits including the previous and current commits), but you can also include a hash in there, e.g. moz-phab submit ebe20e9 4bd2680 included) (push a range of commits from ebe20e9 to 4bd2680 included). See https://github.com/mozilla-conduit/review/blob/master/README.md#submitting-commits-to-the-phabricator.

3 - The first bug I worked on is closed, but when I try to run moz-phab, it still pulls up this commit. Because of this, I have to specify the commit that I want to push for review

Likely what happened is that you kept the local commit you worked on for that bug in your repository. That shouldn't really be a problem though. Once a commit on phabricator is accepted and landed, it ends up on mozilla-central, and then the next time you do hg pull -u central it will end up in your repo. You'll still have the one you pushed to phab, but it'll be its own head. You could remove that local commit though if you wanted, maybe this would help: hg strip -r <hash-of-the-local-commit-you-pushed-to-phab>. You don't need it anymore anyway since that bug is fixed.
I always do this myself. Whenever something gets accepted on phabricator and is going to land, I strip my own commit out of the way.

The problem you described is odd to me, I've not seen it myself. Specifying the exact commit you want to push sounds like the right thing to do.

3 - I made the corrections to the placeholder highlight bug. I performed a commit. But when I try to submit for review with moz-phab, I run into a problem. The commit is listed under another commit for the other bug I'm working on.
I am not sure how to separate the commits or if I should fix the other bug as well and push for review or what I should do. I have the fixes done.

Ok that's odd. moz-phab (or arcanist, or phabricator, not sure which part) uses your commit message to know which bug to associate the review to. So if your message is bug 1493749 - <Description of the bug> then that should automatically link that commit to bug 1493749.
And if you selected only that commit when pushing with moz-phab (e.g. by doing moz-phab submit <hash-of-your-fix>) then I don't see how any other commit would be pushed together with it.
Did you rebase your fix on top of the latest central head?
Maybe that's what you should do. First strip older local commits, update to the latest version of central, and rebase your current fix on top of it, and then push to phabricator again.
To rebase you can do this: hg rebase -s <hash-of-your-commit> -d central.

I hope this can help. If you're still stuck after that, I'm not sure I am going to be the right person to help more. Maybe try one of the IRC channels dedicated to helping with this sort of problems (starting with #introduction, but there's also a #hg channel I think).
If everything fails, feel free to attach a diff file here! That still works fine :) For this, just export your commit with hg export -r <hash-of-your-commit> > some-file.diff and then add it as an attachment here.

Comment 8

[Megan Bailey]

This is extremely helpful. I should be almost done submitting the fixe

Comment 9

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

Hi Megan,
you are assigned to this bug, but there is no activity for a long time.
Please let us know if you have any problems or questions.

Honza

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:nchevobbe, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:nchevobbe, since the bug has recent activity, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 12

[Nicolas Chevobbe [:nchevobbe]]

(clearing ni, bug is still valid)