Spun off from bug 867201. The current disconnect button needs a user confirmation prompt of some sort. Yuan, any suggestions on how to do this in the current (v1) UI?

Mockups for V1 and also V2(for comparison) Tapping on the "Remove" button should perform the remove action. Tapping on cancel will dismiss the inline prompt, display "Remove this device" button. We will use the indeterminate progress ring for syncing, adding, and removing device. In this case, the ring will appear to the left side of action buttons. The mockup is created based on the suggestion I made at Bug 868217. But I think this inline prompt can fit in with our current UI.

Pretty sure Rodrigo has posted a link to the CSS code for this progress indicator somewhere in a bug.

Yes, the details are on Bug 852622.

(In reply to Rodrigo Silveira [:rsilveira] from comment #3) > Yes, the details are on Bug 852622. I think that code is going to need some tweaking. Smaller sizes seemed to make the dots too small.

cleaned up css.

converted everything over to classes, and renamed the css class names so we can reuse this.

I wish we could get better aliasing on this. I wonder if there's some way to soften the outlines of the dots.

I need to know when this is completed so I can display a throbber during the process.

Comment on attachment 745257 [details] [diff] [review] part1 - send obs event when startOver is complete Review of attachment 745257 [details] [diff] [review]: ----------------------------------------------------------------- We use :finish instead of :complete (there are utility functions that do the right thing, too, but no point here).

(In reply to Richard Newman [:rnewman] from comment #10) > Comment on attachment 745257 [details] [diff] [review] > part1 - send obs event when startOver is complete > > Review of attachment 745257 [details] [diff] [review]: > ----------------------------------------------------------------- > > We use :finish instead of :complete (there are utility functions that do the > right thing, too, but no point here). I'll update it to use 'finish' instead.

cleaned up.

Comment on attachment 745657 [details] [diff] [review] part2 - disconnect warn and throbber Review of attachment 745657 [details] [diff] [review]: ----------------------------------------------------------------- r+ with nits. Go forth & ship! ::: browser/metro/base/content/browser.xul @@ +418,4 @@ > </setting> > + <vbox id="sync-disconnectwarnpanel" collapsed="true"> > + <description id="sync-disconnectwarntitle">&sync.removewarn.title;</description> > + <description id="sync-disconnectwarnmsg">&sync.removewarn.note;</description> nit: stray whitespace at the end of sync-disconnectwarnmsg & sync-disconnectwarntitle lines. ::: browser/metro/base/content/sync.js @@ +454,5 @@ > this._elements.errordescription.collapsed = true; > > let isConfigured = (!this._loginError && Weave.Status.checkSetup() != Weave.CLIENT_NOT_CONFIGURED); > > + // If we're in the process of disconnecting, we are not connected I'd modify that comment that 'we are not connected' means we've called Weave.Service.logout() ::: browser/metro/locales/en-US/chrome/sync.dtd @@ +13,5 @@ > <!-- device name text edit --> > <!ENTITY sync.deviceName "This device"> > <!-- remove this device button --> > <!ENTITY sync.removebutton.label "Remove this device"> > +<!ENTITY sync.removewarn.title "Remove Metro Firefox from your Sync Account?"> Please use the &shortBrand names for metrofx & Sync here so that this localizes properly, updates with the correct branding for nightly/aurora/beta, and hurts less if we change the name of the product(s). @@ +20,4 @@ > > <!ENTITY sync.setup2.title "Set up Sync"> > <!ENTITY sync.setup.pair2 "To activate, select &#x0022;Pair a device&#x0022; on your other device."> > + nit: get rid of the space and move the sync.setup entities down with the others. ::: browser/metro/theme/browser.css @@ +740,5 @@ > + width: 22px; > + height: 22px; > +} > + > +#syncdisconnectthrobber .progressBallInner { thanks for writing css classes. Descendant selectors are generally discouraged because they're slow. Well, these are certainly not the worst offenders in our code and this code won't run frequently, so it's not worth the time to make more performant(but fragile) ones.

Comment on attachment 745657 [details] [diff] [review] part2 - disconnect warn and throbber Review of attachment 745657 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/metro/base/content/browser.xul @@ +427,5 @@ > + </vbox> > + <vbox id="sync-disconnectthrobber" collapsed="false"> > + <hbox> > + <spacer flex="1" /> > + <cssthrobber id="syncdisconnectthrobber"></cssthrobber> Nit: <cssthrobber/> works too.

https://hg.mozilla.org/mozilla-central/rev/14e581ecbf0c https://hg.mozilla.org/mozilla-central/rev/616f2609c294

I was looking at sync.properties 1.7 # LOCALIZATION NOTE: Used in the default os description when a new account 1.8 # is being set up. Should be unique to Metro, so that it does not conflict 1.9 # with Desktop. See /services/sync/modules/engines/clients.js locaName. 1.10 sync.defaultAccountApplication=Metro %S 1.11 1.12 +sync.disconnectPrompt=Remove Windows 8 style %S from your Sync Account? As far as I understand both strings should be referring to the same object. Should sync.defaultAccountApplication be changed accordingly? Let me know if you think I should open a new bug for this.

(In reply to Francesco Lodolo [:flod] from comment #18) > I was looking at sync.properties > > 1.7 # LOCALIZATION NOTE: Used in the default os description when a new > account > 1.8 # is being set up. Should be unique to Metro, so that it does not > conflict > 1.9 # with Desktop. See /services/sync/modules/engines/clients.js > locaName. > 1.10 sync.defaultAccountApplication=Metro %S > 1.11 > 1.12 +sync.disconnectPrompt=Remove Windows 8 style %S from your Sync > Account? > > As far as I understand both strings should be referring to the same object. > Should sync.defaultAccountApplication be changed accordingly? Let me know if > you think I should open a new bug for this. No need for a bug. There was a discussion about this on the metro mailing list. The text edit needed a short name that differentiated it from desktop, 'Metro' was chosen.

Thanks Jim, I read the discussion.