Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(125)

Issue 10579026: Fix edit profile link in NTP (Closed)

Created:
8 years, 6 months ago by Evan Stade
Modified:
8 years, 6 months ago
Reviewers:
Dan Beam
CC:
chromium-reviews, arv (Not doing code reviews)
Visibility:
Public.

Description

Fix edit profile link in NTP currently the manage profile overlay is dimissed when new profile data comes in --- instead, just update the overlay. There's a lot of simplification here which is possible because only one profile (the currently active one) may be managed. BUG=132343 TEST=Sign in to Chrome, click username on NTP, edit the current profile. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=143541

Patch Set 1 #

Patch Set 2 : comments #

Patch Set 3 : docs #

Total comments: 18

Patch Set 4 : review changes #

Patch Set 5 : remove dbging line #

Total comments: 10

Patch Set 6 : addressed nitstorm #

Patch Set 7 : more nits #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -85 lines) Patch
M chrome/browser/resources/options2/browser_options.js View 1 2 3 4 5 6 3 chunks +28 lines, -5 lines 2 comments Download
M chrome/browser/resources/options2/manage_profile_overlay.js View 1 2 3 4 5 6 3 chunks +16 lines, -15 lines 0 comments Download
M chrome/browser/ui/webui/options2/manage_profile_handler2.h View 1 chunk +0 lines, -11 lines 0 comments Download
M chrome/browser/ui/webui/options2/manage_profile_handler2.cc View 2 chunks +0 lines, -54 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
Evan Stade
what this does not fix is the flicker as the profile icons load. However it ...
8 years, 6 months ago (2012-06-19 19:10:36 UTC) #1
Dan Beam
http://codereview.chromium.org/10579026/diff/4001/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): http://codereview.chromium.org/10579026/diff/4001/chrome/browser/resources/options2/browser_options.js#newcode1004 chrome/browser/resources/options2/browser_options.js:1004: setProfilesInfo_: function(profiles) { if |profiles| can be null, handle ...
8 years, 6 months ago (2012-06-19 23:44:25 UTC) #2
Evan Stade
http://codereview.chromium.org/10579026/diff/4001/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): http://codereview.chromium.org/10579026/diff/4001/chrome/browser/resources/options2/browser_options.js#newcode1004 chrome/browser/resources/options2/browser_options.js:1004: setProfilesInfo_: function(profiles) { On 2012/06/19 23:44:25, Dan Beam wrote: ...
8 years, 6 months ago (2012-06-20 02:04:31 UTC) #3
Dan Beam
http://codereview.chromium.org/10579026/diff/4001/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): http://codereview.chromium.org/10579026/diff/4001/chrome/browser/resources/options2/browser_options.js#newcode1033 chrome/browser/resources/options2/browser_options.js:1033: return null; On 2012/06/20 02:04:31, Evan Stade wrote: > ...
8 years, 6 months ago (2012-06-20 02:22:06 UTC) #4
Evan Stade
http://codereview.chromium.org/10579026/diff/4001/chrome/browser/resources/options2/manage_profile_overlay.js File chrome/browser/resources/options2/manage_profile_overlay.js (right): http://codereview.chromium.org/10579026/diff/4001/chrome/browser/resources/options2/manage_profile_overlay.js#newcode72 chrome/browser/resources/options2/manage_profile_overlay.js:72: if (location.hash) On 2012/06/20 02:22:07, Dan Beam wrote: > ...
8 years, 6 months ago (2012-06-20 02:24:55 UTC) #5
Dan Beam
http://codereview.chromium.org/10579026/diff/5007/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): http://codereview.chromium.org/10579026/diff/5007/chrome/browser/resources/options2/browser_options.js#newcode1024 chrome/browser/resources/options2/browser_options.js:1024: * @return {Object} A profile info object. {?Object} if ...
8 years, 6 months ago (2012-06-20 02:25:19 UTC) #6
Evan Stade
http://codereview.chromium.org/10579026/diff/5007/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): http://codereview.chromium.org/10579026/diff/5007/chrome/browser/resources/options2/browser_options.js#newcode1024 chrome/browser/resources/options2/browser_options.js:1024: * @return {Object} A profile info object. On 2012/06/20 ...
8 years, 6 months ago (2012-06-20 22:10:06 UTC) #7
Dan Beam
lgtm w/nit http://codereview.chromium.org/10579026/diff/11006/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): http://codereview.chromium.org/10579026/diff/11006/chrome/browser/resources/options2/browser_options.js#newcode1039 chrome/browser/resources/options2/browser_options.js:1039: for (var i = 0; i < ...
8 years, 6 months ago (2012-06-20 22:13:51 UTC) #8
Evan Stade
http://codereview.chromium.org/10579026/diff/11006/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): http://codereview.chromium.org/10579026/diff/11006/chrome/browser/resources/options2/browser_options.js#newcode1039 chrome/browser/resources/options2/browser_options.js:1039: for (var i = 0; i < $('profiles-list').dataModel.length; i++) ...
8 years, 6 months ago (2012-06-20 22:16:16 UTC) #9
Dan Beam
8 years, 6 months ago (2012-06-20 22:21:53 UTC) #10
On 2012/06/20 22:16:16, Evan Stade wrote:
>
http://codereview.chromium.org/10579026/diff/11006/chrome/browser/resources/o...
> File chrome/browser/resources/options2/browser_options.js (right):
> 
>
http://codereview.chromium.org/10579026/diff/11006/chrome/browser/resources/o...
> chrome/browser/resources/options2/browser_options.js:1039: for (var i = 0; i <
> $('profiles-list').dataModel.length; i++) {
> On 2012/06/20 22:13:51, Dan Beam wrote:
> > var model = $('profiles-list').dataModel;
> > // re-use model three time after
> 
> only twice, don't think it's worth it.

twice per loop iteration

Powered by Google App Engine
This is Rietveld 408576698