RESOLVED FIXED Bug 19268
Profiles panel should make recording the first profile more obvious
https://bugs.webkit.org/show_bug.cgi?id=19268
Summary Profiles panel should make recording the first profile more obvious
Adam Roben (:aroben)
Reported 2008-05-27 08:40:32 PDT
The Inspector's Profiles panel should make how to record your first profile more obvious. Currently you're presented with a blank panel. You have to magically know to press the button in the status bar that contains a large gray dot. It would be much better if we had some text like "Record a profile using the Start Profiling button".
Attachments
UI mock (142.86 KB, image/png)
2010-01-13 05:01 PST, Mikhail Naganov
no flags
Proposed patch (17.31 KB, patch)
2010-01-14 08:31 PST, Mikhail Naganov
no flags
Now using '--binary' for localizedStrings (14.10 KB, patch)
2010-01-14 08:35 PST, Mikhail Naganov
timothy: review-
generalized version (14.32 KB, patch)
2010-01-14 10:02 PST, Mikhail Naganov
timothy: review-
comment addressed (14.21 KB, patch)
2010-01-14 12:36 PST, Mikhail Naganov
timothy: review+
timothy: commit-queue-
Mikhail Naganov
Comment 1 2010-01-13 05:01:42 PST
Created attachment 46447 [details] UI mock How about something like this?
Timothy Hatcher
Comment 2 2010-01-13 05:16:44 PST
This seems fine. I don't think we need to have a dot in the text button though. I would reduce the sillouette image a little too, maybe 10%.
Mikhail Naganov
Comment 3 2010-01-14 08:31:23 PST
Created attachment 46572 [details] Proposed patch How it looks like: - with single profile type: http://tinypic.com/r/izss4n/6 - with a couple of profile types: http://tinypic.com/r/2rpy802/6
Mikhail Naganov
Comment 4 2010-01-14 08:35:49 PST
Created attachment 46573 [details] Now using '--binary' for localizedStrings
Timothy Hatcher
Comment 5 2010-01-14 08:54:19 PST
Comment on attachment 46573 [details] Now using '--binary' for localizedStrings > +++ b/WebCore/inspector/front-end/ProfilesWelcomeView.js I think this should not be specific to Profiles panel, and should be generic for use by other Panels. > + registerProfileType: function(profileType) This can be generalized to take just an HTML message, then it isn't Profiles specific. > + } else { > + messageElement.innerHTML = message; > + } No braces here. > +.panel-enabler-view.profiles.welcome { > +.panel-enabler-view.profiles.welcome img { > +.panel-enabler-view.profiles.welcome .instructions { > +.panel-enabler-view.profiles.welcome .profile-type-message { > +.panel-enabler-view.welcome button { These selectors don't need .profiles in them. They can be generic to all welcome views. I think this is fine as, but I would like to see it be more generic.
Mikhail Naganov
Comment 6 2010-01-14 10:02:09 PST
Created attachment 46581 [details] generalized version not adding to commit-queue because it contains a binary diff made by git, which isn't supposed to be successfully applied by svn.
Timothy Hatcher
Comment 7 2010-01-14 10:42:18 PST
Comment on attachment 46581 [details] generalized version > + function messageButtonClicked() > + { > + profileType.buttonClicked.call(profileType); > + } This does not need to use .call(). You can also just use profileType.buttonClicked.bind(profileType) and pass it to addEventListener. Fix that and I'll r+.
Mikhail Naganov
Comment 8 2010-01-14 12:36:11 PST
Created attachment 46598 [details] comment addressed yes, that's simpler
Pavel Feldman
Comment 9 2010-01-15 03:20:43 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/English.lproj/localizedStrings.js M WebCore/WebCore.gypi M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/inspector/front-end/ProfileView.js M WebCore/inspector/front-end/ProfilesPanel.js M WebCore/inspector/front-end/WebKit.qrc A WebCore/inspector/front-end/WelcomeView.js M WebCore/inspector/front-end/inspector.css M WebCore/inspector/front-end/inspector.html Committed r53328
Note You need to log in before you can comment on or make changes to this bug.