Bug 169583 - Web Inspector: RTL: add support for TimelineOverview sidebar and container layout
Summary: Web Inspector: RTL: add support for TimelineOverview sidebar and container la...
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: Devin Rousso
URL:
Keywords:
Depends on:
Blocks: 168287
  Show dependency tree / graph
 
Reported: 2017-03-13 21:21 PDT by Devin Rousso
Modified: 2017-03-14 12:07 PDT (History)
2 users (show)

See Also:

Attachments
Patch (5.22 KB, patch)
2017-03-13 21:32 PDT, Devin Rousso 		bburg: review+
Details | Formatted Diff | Diff
[Image] After Patch is applied (217.25 KB, image/png)
2017-03-13 21:33 PDT, Devin Rousso 		no flags Details
Patch (4.96 KB, patch)
2017-03-14 11:46 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2017-03-13 21:21:59 PDT 
.
Comment 1 Devin Rousso 2017-03-13 21:32:59 PDT 
Created attachment 304345 [details]
Patch
Comment 2 Devin Rousso 2017-03-13 21:33:21 PDT 
Created attachment 304346 [details]
[Image] After Patch is applied
Comment 3 BJ Burg 2017-03-13 22:14:57 PDT 
Comment on attachment 304345 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.css:31
> +body[dir=ltr] .timeline-overview > .tree-outline.timelines {

Can this use :matches() ?

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.css:37
> +body[dir=rtl] .timeline-overview > .tree-outline.timelines {

Ditto

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.css:167
> +    bottom: 0;

Does the ruler still look right in LTR? Or was this property unnecessary?
Comment 4 Devin Rousso 2017-03-13 22:28:20 PDT 
Comment on attachment 304345 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.css:31
>> +body[dir=ltr] .timeline-overview > .tree-outline.timelines {
> 
> Can this use :matches() ?

Good catch!

>> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.css:167
>> +    bottom: 0;
> 
> Does the ruler still look right in LTR? Or was this property unnecessary?

I did this so that I don't have to add a reset for the other property when only setting one or the other in LTR/RTL (see lines 42 and 46).  This way, I can also merge the rules together to make a single selector.
Comment 5 Devin Rousso 2017-03-14 11:46:47 PDT 
Created attachment 304407 [details]
Patch
Comment 6 WebKit Commit Bot 2017-03-14 12:07:32 PDT 
Comment on attachment 304407 [details]
Patch

Clearing flags on attachment: 304407

Committed r213924: <http://trac.webkit.org/changeset/213924>
Comment 7 WebKit Commit Bot 2017-03-14 12:07:35 PDT 
All reviewed patches have been landed.  Closing bug.