Bug 174838

Summary: Web Inspector: Styles: Add pre-populated data to spreadsheet-style view
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, mattbaker, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 145982    
Attachments:
Description Flags
Patch
none
[Image] With patch applied
none
Patch
none
Patch none

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.