https://codereview.appspot.com/220380043/diff/1/trace_viewer/core/analysis/table_builder.html File trace_viewer/core/analysis/table_builder.html (right): https://codereview.appspot.com/220380043/diff/1/trace_viewer/core/analysis/table_builder.html#newcode538 trace_viewer/core/analysis/table_builder.html:538: var descendingArrow = String.fromCharCode(0x25B4); Is there a preferred way ...
10 years, 7 months ago (2015-03-27 18:24:36 UTC) #1
https://codereview.appspot.com/220380043/diff/1/trace_viewer/core/analysis/ta... File trace_viewer/core/analysis/table_builder.html (right): https://codereview.appspot.com/220380043/diff/1/trace_viewer/core/analysis/ta... trace_viewer/core/analysis/table_builder.html:538: var descendingArrow = String.fromCharCode(0x25B4); Is there a preferred way to do this? DESCENDING_ARROW and ASCENDING_ARROW are declared in tracing-analysis-nested-table. So we still pass in the incorrect char, and fix it inside tracing-analysis-header-cell. At least for the table_builder tests, tracing-analysis-header-cell loads before tracing-analysis-nested-table. If I assumed this will always be true I could move DESCENDING_ARROW and ASCENDING_ARROW to be global in tracing-analysis-header-cell instead. But that seems ill-advised since tracing-analysis-nested-table uses one of the arrows for the expandable list. Even if we don't break the table, it makes the code less hackable. The "right" thing to do here seems to be make a new element with the arrows that we use and transitions between each of them. Not sure if that's overkill at this point since the only other place we use the arrows is in tv.c.tracks Thoughts?
Issue 220380043: Fix table_builder to have matching sort arrows, and add transition. Created 10 years, 7 months ago by aiolos Modified 10 years, 5 months ago Reviewers: nduca Base URL: https://github.com/google/trace-viewer@master Comments: 3