Summary: | Web Inspector: RTL: results in Search navigation sidebar have misplaced highlights | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||||||||||||
Component: | Web Inspector | Assignee: | BJ Burg <bburg> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | bburg, commit-queue, inspector-bugzilla-changes, mattbaker, timothy, zalan | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | All | ||||||||||||||||||
OS: | All | ||||||||||||||||||
Attachments: |
|
Description
BJ Burg
2017-03-30 12:34:02 PDT
Created attachment 305879 [details]
Patch
Created attachment 305880 [details]
Before [RTL]
Created attachment 305881 [details]
after [RTL]
Comment on attachment 305879 [details]
Patch
Found an issue when the result line is too big.
Created attachment 305886 [details]
Patch
Comment on attachment 305886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=305886&action=review > Source/WebInspectorUI/UserInterface/Views/SearchIcons.css:50 > +.source-code-match .icon { Nice cleanup, so tired of seeing .yet-another-icon .icon everywhere. > Source/WebInspectorUI/UserInterface/Views/SearchResultTreeElement.js:42 > + const charactersToShowBeforeSearchMatch = isRTL ? 20 : 15; Why is this changing to 20? It feels wrong presenting less/different data in RTL. > Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.css:57 > + text-align: right; This looks wrong. Search results are now right aligned when viewed in LTR. (In reply to Matt Baker from comment #6) > Comment on attachment 305886 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=305886&action=review > > > Source/WebInspectorUI/UserInterface/Views/SearchIcons.css:50 > > +.source-code-match .icon { > > Nice cleanup, so tired of seeing .yet-another-icon .icon everywhere. > > > Source/WebInspectorUI/UserInterface/Views/SearchResultTreeElement.js:42 > > + const charactersToShowBeforeSearchMatch = isRTL ? 20 : 15; > > Why is this changing to 20? It feels wrong presenting less/different data in > RTL. We have to be more conservative so the highlight text doesn't run off into overflow. It's hard to determine what will actually show in RTL since we don't know how much text might overflow to the left, whereas in LTR we know exactly what text starts on the left side. This value worked for me when resizing the sidebar to the minimum expanded width. > > Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.css:57 > > + text-align: right; > > This looks wrong. Search results are now right aligned when viewed in LTR. Will fix. I'm tempted to just leave subtitles out since we can't find a good way to overflow them without making search results impossible to consistently align left. If the tooltip still displays the title and subtitle, that might be fine. Created attachment 306058 [details]
Patch v1.1
Created attachment 306059 [details]
search results - LTR
Created attachment 306060 [details]
search results - RTL
(In reply to Brian Burg from comment #7) > > I'm tempted to just leave subtitles out since we can't find a good way to > overflow them without making search results impossible to consistently align > left. If the tooltip still displays the title and subtitle, that might be > fine. Oops, this comment belongs in a different bug. Comment on attachment 306058 [details] Patch v1.1 Clearing flags on attachment: 306058 Committed r214864: <http://trac.webkit.org/changeset/214864> All reviewed patches have been landed. Closing bug. |