Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(511)

Issue 1262063003: Devtools UI: Remove redundant tooltips (Closed)

Created:
5 years, 4 months ago by samli
Modified:
5 years, 3 months ago
Reviewers:
paulirish, dgozman, pfeldman
CC:
apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, sergeyv+blink_chromium.org, yurys+blink_chromium.org
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Devtools UI: Remove redundant tooltips Tooltips should be used where they add value to the the user. This change removes instances where tooltips are used with the same text as is already displayed. BUG=513066 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201577

Patch Set 1 #

Patch Set 2 : #

Total comments: 12

Patch Set 3 : #

Total comments: 7

Patch Set 4 : #

Patch Set 5 : #

Total comments: 8

Patch Set 6 : #

Total comments: 6

Patch Set 7 : #

Patch Set 8 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -8 lines) Patch
M Source/devtools/front_end/components/Linkifier.js View 1 2 3 4 5 7 1 chunk +2 lines, -2 lines 0 comments Download
M Source/devtools/front_end/elements/ComputedStyleWidget.js View 1 2 3 4 5 6 7 1 chunk +1 line, -3 lines 0 comments Download
M Source/devtools/front_end/elements/StylesSidebarPane.js View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M Source/devtools/front_end/ui/treeoutline.js View 1 2 3 4 5 6 7 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 26 (5 generated)
samli
PTAL. - for Linkifier/ResourceUtils, I wanted to switch them to Tooltips but they are in ...
5 years, 4 months ago (2015-07-30 07:06:16 UTC) #2
paulirish
A little unclear about removing some of these. https://codereview.chromium.org/1262063003/diff/20001/Source/devtools/front_end/components/DOMPresentationUtils.js File Source/devtools/front_end/components/DOMPresentationUtils.js (left): https://codereview.chromium.org/1262063003/diff/20001/Source/devtools/front_end/components/DOMPresentationUtils.js#oldcode76 Source/devtools/front_end/components/DOMPresentationUtils.js:76: parentElement.title ...
5 years, 4 months ago (2015-07-30 07:20:49 UTC) #4
samli
https://codereview.chromium.org/1262063003/diff/20001/Source/devtools/front_end/components/DOMPresentationUtils.js File Source/devtools/front_end/components/DOMPresentationUtils.js (left): https://codereview.chromium.org/1262063003/diff/20001/Source/devtools/front_end/components/DOMPresentationUtils.js#oldcode76 Source/devtools/front_end/components/DOMPresentationUtils.js:76: parentElement.title = title; On 2015/07/30 at 07:20:49, paulirish wrote: ...
5 years, 4 months ago (2015-07-30 08:02:33 UTC) #5
dgozman
I'm not sure how I'm supposed to review non-obvious feature removing without explanation. Also, how ...
5 years, 4 months ago (2015-07-30 10:31:20 UTC) #6
samli
> Also, how does removing tooltips align with issue "Add descriptive tooltips to the UI"? ...
5 years, 4 months ago (2015-08-03 01:16:26 UTC) #7
dgozman
On 2015/08/03 01:16:26, samli wrote: > > Also, how does removing tooltips align with issue ...
5 years, 4 months ago (2015-08-03 07:46:02 UTC) #8
paulirish
Can you attach screenshots for the DOMPresentation & ElementsBreadcrumbs titles you've removed? https://codereview.chromium.org/1262063003/diff/40001/Source/devtools/front_end/components/NetworkConditionsSelector.js File Source/devtools/front_end/components/NetworkConditionsSelector.js ...
5 years, 4 months ago (2015-08-03 16:26:20 UTC) #9
samli
https://codereview.chromium.org/1262063003/diff/40001/Source/devtools/front_end/components/NetworkConditionsSelector.js File Source/devtools/front_end/components/NetworkConditionsSelector.js (left): https://codereview.chromium.org/1262063003/diff/40001/Source/devtools/front_end/components/NetworkConditionsSelector.js#oldcode48 Source/devtools/front_end/components/NetworkConditionsSelector.js:48: option.title = WebInspector.UIString("Maximum download throughput: %s.\r\nMinimum round-trip time: %dms.", ...
5 years, 4 months ago (2015-08-05 07:53:31 UTC) #10
samli
PTAL. Only a few left now. https://codereview.chromium.org/1262063003/diff/20001/Source/devtools/front_end/components/DOMPresentationUtils.js File Source/devtools/front_end/components/DOMPresentationUtils.js (left): https://codereview.chromium.org/1262063003/diff/20001/Source/devtools/front_end/components/DOMPresentationUtils.js#oldcode76 Source/devtools/front_end/components/DOMPresentationUtils.js:76: parentElement.title = title; ...
5 years, 4 months ago (2015-08-06 03:51:16 UTC) #11
paulirish
> This is not that and still works. The only instances I could find where ...
5 years, 4 months ago (2015-08-11 00:52:25 UTC) #12
samli
ping dgozman
5 years, 4 months ago (2015-08-11 02:00:09 UTC) #13
dgozman
https://codereview.chromium.org/1262063003/diff/80001/Source/devtools/front_end/bindings/ResourceUtils.js File Source/devtools/front_end/bindings/ResourceUtils.js (right): https://codereview.chromium.org/1262063003/diff/80001/Source/devtools/front_end/bindings/ResourceUtils.js#newcode182 Source/devtools/front_end/bindings/ResourceUtils.js:182: else if (tooltipText && tooltipText.length) Just |if (tooltipText)| since ...
5 years, 4 months ago (2015-08-12 17:19:34 UTC) #14
samli
https://codereview.chromium.org/1262063003/diff/80001/Source/devtools/front_end/bindings/ResourceUtils.js File Source/devtools/front_end/bindings/ResourceUtils.js (right): https://codereview.chromium.org/1262063003/diff/80001/Source/devtools/front_end/bindings/ResourceUtils.js#newcode182 Source/devtools/front_end/bindings/ResourceUtils.js:182: else if (tooltipText && tooltipText.length) On 2015/08/12 at 17:19:34, ...
5 years, 4 months ago (2015-08-17 00:53:26 UTC) #15
samli
https://codereview.chromium.org/1262063003/diff/80001/Source/devtools/front_end/elements/StylesSidebarPane.js File Source/devtools/front_end/elements/StylesSidebarPane.js (left): https://codereview.chromium.org/1262063003/diff/80001/Source/devtools/front_end/elements/StylesSidebarPane.js#oldcode2285 Source/devtools/front_end/elements/StylesSidebarPane.js:2285: this.nameElement.title = this.property.propertyText; On 2015/08/17 at 00:53:26, samli wrote: ...
5 years, 4 months ago (2015-08-18 04:10:29 UTC) #16
samli
ping +pfeldman
5 years, 4 months ago (2015-08-19 05:30:29 UTC) #18
pfeldman
https://codereview.chromium.org/1262063003/diff/100001/Source/devtools/front_end/components/Linkifier.js File Source/devtools/front_end/components/Linkifier.js (right): https://codereview.chromium.org/1262063003/diff/100001/Source/devtools/front_end/components/Linkifier.js#newcode513 Source/devtools/front_end/components/Linkifier.js:513: if (!tooltipText && linkText !== url) What about the ...
5 years, 4 months ago (2015-08-19 19:08:42 UTC) #19
samli
https://codereview.chromium.org/1262063003/diff/100001/Source/devtools/front_end/components/Linkifier.js File Source/devtools/front_end/components/Linkifier.js (right): https://codereview.chromium.org/1262063003/diff/100001/Source/devtools/front_end/components/Linkifier.js#newcode513 Source/devtools/front_end/components/Linkifier.js:513: if (!tooltipText && linkText !== url) On 2015/08/19 at ...
5 years, 4 months ago (2015-08-21 01:31:47 UTC) #20
samli
ping
5 years, 3 months ago (2015-08-31 22:03:32 UTC) #21
pfeldman
lgtm
5 years, 3 months ago (2015-09-01 19:25:16 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1262063003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1262063003/140001
5 years, 3 months ago (2015-09-01 21:19:25 UTC) #25
commit-bot: I haz the power
5 years, 3 months ago (2015-09-01 22:21:24 UTC) #26
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201577

Powered by Google App Engine
This is Rietveld 408576698