If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[webide] "reload" button

RESOLVED FIXED in Firefox 35

Status

()

Firefox
Developer Tools: WebIDE
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: paul, Assigned: ednapiranha)

Tracking

Trunk
Firefox 35
x86_64
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Comment hidden (empty)
(Reporter)

Comment 1

3 years ago
We want the play button to have 2 icons: play and reload.
Summary: [webide] → [webide] "reload" button
(Reporter)

Comment 2

3 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

3 years ago
Flags: needinfo?(dhenein)
Assignee: nobody → shorlander
Flags: needinfo?(dhenein)
(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.
Flags: needinfo?(paul)
(Reporter)

Comment 4

3 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)
Created attachment 8494528 [details]
icons.png - 01

Added Reload Icon to the sprite sheet.
Flags: needinfo?(paul)
(Reporter)

Comment 6

3 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

3 years ago
Created attachment 8495594 [details] [diff] [review]
bug1057958.patch
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)
Created attachment 8496101 [details]
icons.png - 02

Oops, sorry about that.
Attachment #8494528 - Attachment is obsolete: true
Flags: needinfo?(shorlander)
(Assignee)

Comment 13

3 years ago
Created attachment 8496192 [details] [diff] [review]
bug1057958.patch
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

3 years ago
Created attachment 8497402 [details] [diff] [review]
bug1057958.patch

small css update
Attachment #8496192 - Attachment is obsolete: true
(Assignee)

Comment 16

3 years ago
Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=983b767d7b91
(Assignee)

Updated

3 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

3 years ago
Attachment #8497402 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/076790f9ebc2
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/076790f9ebc2
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
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)
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)
(Assignee)

Comment 22

3 years ago
I don't see why not if it's not too late.
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)
Oops. Not sure what I was smoking this morning but yeah it's on 35 already. :)  Good enough!
You need to log in before you can comment on or make changes to this bug.