Summary: | [results.webkit.org] Timeline.CanvasXAxisComponent height should be defined by option | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zhifei Fang <zhifei_fang> | ||||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, jbedard, webkit-bug-importer, zhifei_fang | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | Safari Technology Preview | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Zhifei Fang
2019-07-31 14:54:29 PDT
Created attachment 375264 [details]
Patch
Created attachment 375265 [details]
Patch
Comment on attachment 375265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375265&action=review > Tools/ChangeLog:9 > + Let the axis' height can be defined in the option. time line Nit: 'Let the axis' height be defined in the option object.' > Tools/ChangeLog:10 > + Compoenent will read these height and automatically set the height for I think this should be 'The timeline component will use this value to set the padding-top for headers, which allows headers to start in the correct position even with multiple top axises.' > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:512 > + const canvasHeight = typeof option.height === "number" ? option.height : parseInt(computedStyle.getPropertyValue('--smallSize')) * 5; This changer makes the default canvasHeight larger too, right? (In reply to Jonathan Bedard from comment #3) > Comment on attachment 375265 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=375265&action=review > > > Tools/ChangeLog:9 > > + Let the axis' height can be defined in the option. time line > > Nit: 'Let the axis' height be defined in the option object.' > > > Tools/ChangeLog:10 > > + Compoenent will read these height and automatically set the height for > > I think this should be 'The timeline component will use this value to set > the padding-top for headers, which allows headers to start in the correct > position even with multiple top axises.' > > > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:512 > > + const canvasHeight = typeof option.height === "number" ? option.height : parseInt(computedStyle.getPropertyValue('--smallSize')) * 5; > > This changer makes the default canvasHeight larger too, right? Yes Created attachment 375322 [details]
Patch
Comment on attachment 375322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375322&action=review > Tools/ChangeLog:9 > + Let the axis' height be defined in the option. Timeine compoenent will use this value to Nit: 'component' Nit: 'use this value as padding-top' Created attachment 375342 [details]
Patch
Comment on attachment 375342 [details] Patch Clearing flags on attachment: 375342 Committed r248134: <https://trac.webkit.org/changeset/248134> All reviewed patches have been landed. Closing bug. |