Bug 174838 - Web Inspector: Styles: Add pre-populated data to spreadsheet-style view
Summary: Web Inspector: Styles: Add pre-populated data to spreadsheet-style view
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks: 145982
  Show dependency treegraph
 
Reported: 2017-07-25 15:06 PDT by Nikita Vasilyev
Modified: 2017-08-09 16:56 PDT (History)
4 users (show)

See Also:


Attachments
Patch (10.06 KB, patch)
2017-08-04 12:49 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
[Image] With patch applied (316.08 KB, image/png)
2017-08-08 16:28 PDT, Nikita Vasilyev
no flags Details
Patch (10.18 KB, patch)
2017-08-08 16:30 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Patch (1.88 KB, patch)
2017-08-09 16:55 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2017-07-25 15:06:44 PDT
Implement a spreadsheet-style view. Use a hardcoded data for now.
Comment 1 Radar WebKit Bug Importer 2017-07-25 15:07:00 PDT
<rdar://problem/33523220>
Comment 2 Nikita Vasilyev 2017-08-04 12:49:43 PDT
Created attachment 317270 [details]
Patch

Same as a static HTML page:
http://nv.github.io/webkit-inspector-bugs/styles-redesign/styles.html
Comment 3 Matt Baker 2017-08-08 12:29:14 PDT
The style/layout of the static demo page looks good. What is the goal of this patch? What is the benefit of using temporary data, instead of hooking up the new UI to existing data in pieces?
Comment 4 Nikita Vasilyev 2017-08-08 14:20:04 PDT
Hooking up the new UI to existing data, even in pieces, is more time consuming process. I wanted to make
sure there's nothing conceptually wrong in UI and we're on the same page, before I spend too much time working on it.

I'll precede hooking up the UI to existing data when this patch lands.
Comment 5 Nikita Vasilyev 2017-08-08 14:49:08 PDT
> I'll precede hooking up the UI to existing data when this patch lands.

... in Bug 175343 - Web Inspector: Styles Redesign: hook up real data to spreadsheet style editor
Comment 6 Matt Baker 2017-08-08 14:50:18 PDT
Comment on attachment 317270 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317270&action=review

> Source/WebInspectorUI/ChangeLog:9
> +        Add a static HTML of three hard-coded CSS rules.

A brief description of what this patch is for would help. Maybe start with something like: "This patch adds static content to the experimental RulesStyleSpreadsheetDetailsPanel..."

> Source/WebInspectorUI/UserInterface/Views/RulesStyleSpreadsheetDetailsPanel.css:38
> +    float: left;

Make sure this layout works in both LTR and RTL. I think this would be a good thing to do from the very beginning, during this prototyping phase.

> Source/WebInspectorUI/UserInterface/Views/RulesStyleSpreadsheetDetailsPanel.css:74
> +    padding-left: 16px;

RTL support.

> Source/WebInspectorUI/UserInterface/Views/RulesStyleSpreadsheetDetailsPanel.css:87
> +    float: right;

RTL support.

> Source/WebInspectorUI/UserInterface/Views/RulesStyleSpreadsheetDetailsPanel.css:113
> +    clear: left;

RTL support.

> Source/WebInspectorUI/UserInterface/Views/RulesStyleSpreadsheetDetailsPanel.css:121
> +    left: 0;

RTL support.
Comment 7 Matt Baker 2017-08-08 14:51:23 PDT
Please add screenshots showing the new UI in the Inspector. A RTL screenshot would be great.
Comment 8 Nikita Vasilyev 2017-08-08 16:25:22 PDT
Adding RTL support isn't straightforward. I think it should be a separate task.

CSS code is LTR. We don't align right anything in the current styles sidebar. See rdar://problem/31961976.

I think we can do a way better job with RTL using a spreadsheet model. We can flip name and value cells, for example. That being said, it isn't a trivial as replacing all "left" properties with right.
Comment 9 Nikita Vasilyev 2017-08-08 16:28:19 PDT
Created attachment 317642 [details]
[Image] With patch applied
Comment 10 Nikita Vasilyev 2017-08-08 16:30:21 PDT
Created attachment 317643 [details]
Patch

I only updated the changelog description.
Comment 11 Nikita Vasilyev 2017-08-08 16:47:25 PDT
I (In reply to Nikita Vasilyev from comment #8)
> Adding RTL support isn't straightforward. I think it should be a separate
> task.

I just filed Bug 175357 - Web Inspector: Styles redesign: add RTL support
Comment 12 Matt Baker 2017-08-08 16:54:57 PDT
Comment on attachment 317643 [details]
Patch

r=me
Comment 13 WebKit Commit Bot 2017-08-09 10:46:30 PDT
Comment on attachment 317643 [details]
Patch

Clearing flags on attachment: 317643

Committed r220462: <http://trac.webkit.org/changeset/220462>
Comment 14 WebKit Commit Bot 2017-08-09 10:46:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Nikita Vasilyev 2017-08-09 16:55:15 PDT
Created attachment 317759 [details]
Patch
Comment 16 Nikita Vasilyev 2017-08-09 16:56:28 PDT
Comment on attachment 317759 [details]
Patch

I'm sorry, wrong browser tab.