WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
168446
Web Inspector: RTL: add TabBar support for RTL layout
https://bugs.webkit.org/show_bug.cgi?id=168446
Summary
Web Inspector: RTL: add TabBar support for RTL layout
Blaze Burg
Reported
2017-02-16 11:00:15 PST
It's a bit messy.
Attachments
Patch
(16.52 KB, patch)
2017-02-16 11:28 PST
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
After - RTL
(4.61 MB, video/quicktime)
2017-02-16 11:33 PST
,
Blaze Burg
no flags
Details
Patch v2
(17.67 KB, patch)
2017-02-17 14:25 PST
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2017-02-16 11:01:08 PST
<
rdar://problem/30559805
>
Blaze Burg
Comment 2
2017-02-16 11:28:04 PST
Created
attachment 301787
[details]
Patch
Blaze Burg
Comment 3
2017-02-16 11:33:44 PST
Created
attachment 301789
[details]
After - RTL To Test: - Select different tabs, observe correct border and background color changes - Observe correct static styles when window is inactive - Drag a tab over other tabs, ensure correct styles and tab moves - Close a tab using [X] button, tab bar shouldn't animate until mouseout of tab bar - Close a tab should animate other tabs cleanly - Adding a new tab with [+] button should animate cleanly from the left
Devin Rousso
Comment 4
2017-02-16 11:55:46 PST
Comment on
attachment 301787
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=301787&action=review
> Source/WebInspectorUI/UserInterface/Views/TabBar.css:109 > +body[dir=ltr] .tab-bar > :nth-child(n + 2 of .item),
NIT: should we put quotes around the attribute value? I think that's a bit clearer. body[dir="ltr"] .tab-bar > :nth-child(n + 2 of .item),
Blaze Burg
Comment 5
2017-02-16 21:28:46 PST
(In reply to
comment #4
)
> Comment on
attachment 301787
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=301787&action=review
> > > Source/WebInspectorUI/UserInterface/Views/TabBar.css:109 > > +body[dir=ltr] .tab-bar > :nth-child(n + 2 of .item), > > NIT: should we put quotes around the attribute value? I think that's a bit > clearer. > > body[dir="ltr"] .tab-bar > :nth-child(n + 2 of .item),
I don't think it actually matters. You are only ever going to see dir=ltr or dir=rtl on the body selector, so you quickly figure out that it's another spammy rule for RTL support =)
Matt Baker
Comment 6
2017-02-17 00:44:53 PST
Comment on
attachment 301787
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=301787&action=review
r- for the layout issue, looks good otherwise. I really like the use of local CSS variables, rather than polluting the global namespace.
> Source/WebInspectorUI/UserInterface/Views/TabBar.css:53 > + --tab-item-light-border-style: 1px solid var(--tab-item-medium-border-color);
Shouldn't this be `var(--tab-item-light-border-color)`?
> Source/WebInspectorUI/UserInterface/Views/TabBar.css:310 > + --tab-item-collapsed-close-button-margin-end: 0;
This variable isn't used anywhere. It should be `--tab-item-close-button-margin-end`, so that it overrides that value for collapsed tabs. Otherwise, hovering the tab causes the close button to appear and bump the layout over 4px.
> Source/WebInspectorUI/UserInterface/Views/TabBar.js:416 > + return WebInspector.resolvedLayoutDirection() === WebInspector.LayoutDirection.LTR ? this._tabBarItems : this._tabBarItems.slice().reverse();
Nice.
Blaze Burg
Comment 7
2017-02-17 14:25:52 PST
Created
attachment 301988
[details]
Patch v2
WebKit Commit Bot
Comment 8
2017-02-17 14:56:36 PST
Comment on
attachment 301988
[details]
Patch v2 Clearing flags on attachment: 301988 Committed
r212582
: <
http://trac.webkit.org/changeset/212582
>
WebKit Commit Bot
Comment 9
2017-02-17 14:56:42 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug