Summary: | [results.webkit.org Timeline] Add notify rerender API for timeline | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zhifei Fang <zhifei_fang> | ||||||||
Component: | Tools / Tests | Assignee: | Zhifei Fang <zhifei_fang> | ||||||||
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-08-30 15:58:12 PDT
Created attachment 377761 [details]
Patch
Created attachment 377766 [details]
Patch
Comment on attachment 377766 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377766&action=review > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:137 > + const layoutSizeMayChange = new EventStream(); Feel like this should be something like 'onResizeActions' or 'onLayoutChangeActions' > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:149 > + /** What prompted this new comment format? I know it's pretty common in projects which have some sort of auto-documenter, but we don't have that in WebKit. > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:155 > + // this make sure the newly added children receive current state I would say: 'Propigate the current state to new children' > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:820 > + Nit: Extra newline? > Tools/resultsdbpy/resultsdbpy/view/templates/search.html:45 > + Nit: Extra newline > Tools/resultsdbpy/resultsdbpy/view/templates/search.html:220 > +const layoutSizeMayChange = new EventStream(); Feel like this should be something like 'onResizeActions' or 'onLayoutChangeActions' > Tools/resultsdbpy/resultsdbpy/view/templates/search.html:227 > + Nit: Extra newline Comment on attachment 377766 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377766&action=review > Tools/resultsdbpy/resultsdbpy/view/templates/search.html:238 > + ]), () => layoutSizeMayChange.add()} Can you double-check parenthesis here? I think this broke the sidebar on my local instance. Created attachment 377772 [details]
Patch
(In reply to Jonathan Bedard from comment #3) > Comment on attachment 377766 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377766&action=review > > > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:137 > > + const layoutSizeMayChange = new EventStream(); > > Feel like this should be something like 'onResizeActions' or > 'onLayoutChangeActions' > > > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:149 > > + /** > > What prompted this new comment format? I know it's pretty common in projects > which have some sort of auto-documenter, but we don't have that in WebKit. > > > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:155 > > + // this make sure the newly added children receive current state > > I would say: > 'Propigate the current state to new children' > > > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:820 > > + > > Nit: Extra newline? > > > Tools/resultsdbpy/resultsdbpy/view/templates/search.html:45 > > + > > Nit: Extra newline > > > Tools/resultsdbpy/resultsdbpy/view/templates/search.html:220 > > +const layoutSizeMayChange = new EventStream(); > > Feel like this should be something like 'onResizeActions' or > 'onLayoutChangeActions' > > > Tools/resultsdbpy/resultsdbpy/view/templates/search.html:227 > > + > > Nit: Extra newline My new API's are all anonynous functions, so I'm adding docstrings to give them a name so that their purpose is clear Comment on attachment 377772 [details] Patch Clearing flags on attachment: 377772 Committed r249358: <https://trac.webkit.org/changeset/249358> All reviewed patches have been landed. Closing bug. |