Closed
Bug 1057958
Opened 10 years ago
Closed 10 years ago
[webide] "reload" button
Categories
(DevTools Graveyard :: WebIDE, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 35
People
(Reporter: paul, Assigned: jfong)
Details
Attachments
(2 files, 3 obsolete files)
28.92 KB,
image/png
|
Details | |
79.54 KB,
patch
|
jfong
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•10 years ago
|
||
We want the play button to have 2 icons: play and reload.
Summary: [webide] → [webide] "reload" button
Reporter | ||
Comment 2•10 years ago
|
||
Darrin, the same way we have play/stop/pause, can we get a "reload" icon? http://mxr.mozilla.org/mozilla-central/source/browser/devtools/webide/themes/icons.png
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(dhenein)
Updated•10 years ago
|
Assignee: nobody → shorlander
Flags: needinfo?(dhenein)
Comment 3•10 years ago
|
||
(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #2) > Darrin, the same way we have play/stop/pause, can we get a "reload" icon? > http://mxr.mozilla.org/mozilla-central/source/browser/devtools/webide/themes/ > icons.png Do you want me to try and add it to that sprite? If I follow that format it will break all the coordinates.
Updated•10 years ago
|
Flags: needinfo?(paul)
Reporter | ||
Comment 4•10 years ago
|
||
(In reply to Stephen Horlander [:shorlander] from comment #3) > (In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from > comment #2) > > Darrin, the same way we have play/stop/pause, can we get a "reload" icon? > > http://mxr.mozilla.org/mozilla-central/source/browser/devtools/webide/themes/ > > icons.png > > Do you want me to try and add it to that sprite? If I follow that format it > will break all the coordinates. Not a problem. Thank you!
Flags: needinfo?(paul)
Reporter | ||
Comment 6•10 years ago
|
||
(In reply to Stephen Horlander [:shorlander] from comment #5) > Created attachment 8494528 [details] > icons.png - 01 > > Added Reload Icon to the sprite sheet. Perfect! Thank you so much!!!
Flags: needinfo?(paul)
Jen, can you take a look at this? I should involve: * Updating the sprite map and coordinates for icons * Changing the play button to switch to the reload icon once the app is already launched / running
Assignee: shorlander → jfong
Status: NEW → ASSIGNED
"It should involve..."
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8495594 -
Flags: review?(jryans)
Comment on attachment 8495594 [details] [diff] [review] bug1057958.patch Review of attachment 8495594 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for doing this, especially with the sprite sheet. I know it's annoying to update all the locations... :/ However, I just noticed that the new image seems to have removed the blue phone icon that's used once the runtime in connected. So, we'll need Shorlander to put that back first.
Attachment #8495594 -
Flags: review?(jryans)
Shorlander, please restore the blue phone icon in the new sprite sheet.
Flags: needinfo?(shorlander)
Comment 12•10 years ago
|
||
Oops, sorry about that.
Attachment #8494528 -
Attachment is obsolete: true
Flags: needinfo?(shorlander)
Assignee | ||
Comment 13•10 years ago
|
||
Attachment #8495594 -
Attachment is obsolete: true
Attachment #8496192 -
Flags: review?(jryans)
Comment on attachment 8496192 [details] [diff] [review] bug1057958.patch Review of attachment 8496192 [details] [diff] [review]: ----------------------------------------------------------------- Great! Looks good to me.
Attachment #8496192 -
Flags: review?(jryans) → review+
Assignee | ||
Comment 15•10 years ago
|
||
small css update
Attachment #8496192 -
Attachment is obsolete: true
Assignee | ||
Comment 16•10 years ago
|
||
Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=983b767d7b91
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
(In reply to Jen Fong-Adwent [:ednapiranha] from comment #15) > Created attachment 8497402 [details] [diff] [review] > bug1057958.patch > > small css update With a small change like this after you've already gotten r+, it's good practice to "carry over" the r+ and set it yourself on the new patch. This way, the sheriffs who land things when you set "checkin-needed" can be sure there was a review at some point.
Assignee | ||
Updated•10 years ago
|
Attachment #8497402 -
Flags: review+
Comment 18•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/076790f9ebc2
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Comment 19•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/076790f9ebc2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
Comment 20•10 years ago
|
||
Shouldn't this be uplifted to Firefox 34 beta so that there are no ambiguous messages when one accidentally wants to add the same app twice?
Flags: needinfo?(jryans)
Comment 21•10 years ago
|
||
Petruta, I'm not sure if it's too late for that or not. Jen, do you want to nominate this for uplift to 34, or maybe 35?
Flags: needinfo?(jfong)
As this is mainly a usability improvement and not a regression or bug fix, it seems more appropriate to let it ride the trains. It landed with 35, so it's already there in Dev. Edition. I think that's good enough. If others feel more strongly though, feel free to request beta uplift. :)
Flags: needinfo?(jryans)
Comment 24•10 years ago
|
||
Oops. Not sure what I was smoking this morning but yeah it's on 35 already. :) Good enough!
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•4 years ago
|
Product: DevTools → DevTools Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•