Improve API and documentation of ColumnChart Used to be "BarChart" and some remnants of "bar" still existed.
Created attachment 360500 [details] [PATCH] Proposed Fix
Comment on attachment 360500 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=360500&action=review r=me > Source/WebInspectorUI/ChangeLog:8 > + This used to be named "BarChart", convert remaining instances Grammar: this sentence is a fragment, so use a ";" or add some other conjunction. > Source/WebInspectorUI/UserInterface/Views/ColumnChart.js:59 > + get columns() { return this._columns; } Considering that nobody calls this (and frankly the data it contains is really not very useful), I think we should just remove this. > Source/WebInspectorUI/UserInterface/Views/ColumnChart.js:82 > + this._columns = []; I feel like this should call `this.needsLayout()` 🤔 > Source/WebInspectorUI/UserInterface/Views/ColumnChart.js:85 > needsLayout() Perhaps this class should be a subclass of `WI.View`? > Source/WebInspectorUI/UserInterface/Views/StackedLineChart.js:63 > + get element() { return this._element; } Yes! ᕦ(ò_óˇ)ᕤ
(In reply to Devin Rousso from comment #2) > Comment on attachment 360500 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=360500&action=review > > r=me > > > Source/WebInspectorUI/ChangeLog:8 > > + This used to be named "BarChart", convert remaining instances > > Grammar: this sentence is a fragment, so use a ";" or add some other > conjunction. kk. > > Source/WebInspectorUI/UserInterface/Views/ColumnChart.js:59 > > + get columns() { return this._columns; } > > Considering that nobody calls this (and frankly the data it contains is > really not very useful), I think we should just remove this. Okay. > > Source/WebInspectorUI/UserInterface/Views/ColumnChart.js:85 > > needsLayout() > > Perhaps this class should be a subclass of `WI.View`? Will do in a bit.
https://trac.webkit.org/changeset/240865/webkit
<rdar://problem/47747238>