Bug 19268 - Profiles panel should make recording the first profile more obvious
: Profiles panel should make recording the first profile more obvious
Status: RESOLVED FIXED
: WebKit
Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2008-05-27 08:40 PST by
Modified: 2010-01-15 03:20 PST (History)


Attachments
UI mock (142.86 KB, image/png)
2010-01-13 05:01 PST, Mikhail Naganov
no flags Details
Proposed patch (17.31 KB, patch)
2010-01-14 08:31 PST, Mikhail Naganov
no flags Review Patch | Details | Formatted Diff | Diff
Now using '--binary' for localizedStrings (14.10 KB, patch)
2010-01-14 08:35 PST, Mikhail Naganov
timothy: review-
Review Patch | Details | Formatted Diff | Diff
generalized version (14.32 KB, patch)
2010-01-14 10:02 PST, Mikhail Naganov
timothy: review-
Review Patch | Details | Formatted Diff | Diff
comment addressed (14.21 KB, patch)
2010-01-14 12:36 PST, Mikhail Naganov
timothy: review+
timothy: commit‑queue-
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-05-27 08:40:32 PST
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".
------- Comment #1 From 2010-01-13 05:01:42 PST -------
Created an attachment (id=46447) [details]
UI mock

How about something like this?
------- Comment #2 From 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%.
------- Comment #3 From 2010-01-14 08:31:23 PST -------
Created an attachment (id=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
------- Comment #4 From 2010-01-14 08:35:49 PST -------
Created an attachment (id=46573) [details]
Now using '--binary' for localizedStrings
------- Comment #5 From 2010-01-14 08:54:19 PST -------
(From update of attachment 46573 [details])

> +++ 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.
------- Comment #6 From 2010-01-14 10:02:09 PST -------
Created an attachment (id=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.
------- Comment #7 From 2010-01-14 10:42:18 PST -------
(From update of attachment 46581 [details])

> +            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+.
------- Comment #8 From 2010-01-14 12:36:11 PST -------
Created an attachment (id=46598) [details]
comment addressed

yes, that's simpler
------- Comment #9 From 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