Bug 201363 - [results.webkit.org Timeline] Add notify rerender API for timeline
Summary: [results.webkit.org Timeline] Add notify rerender API for timeline
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zhifei Fang
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-30 15:58 PDT by Zhifei Fang
Modified: 2019-08-30 21:12 PDT (History)
4 users (show)

See Also:


Attachments
Patch (8.79 KB, patch)
2019-08-30 16:02 PDT, Zhifei Fang
no flags Details | Formatted Diff | Diff
Patch (13.06 KB, patch)
2019-08-30 16:25 PDT, Zhifei Fang
no flags Details | Formatted Diff | Diff
Patch (12.73 KB, patch)
2019-08-30 17:06 PDT, Zhifei Fang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zhifei Fang 2019-08-30 15:58:12 PDT
Because currently we don't have the resize observer implemented, I provide this API for user to call when his code change the layout and may affect the timeline width.

This API will let the timeline self adapt to it's parent flex box width, or user can give a fixed width, then the timeline will re-render to that specify width.
Comment 1 Zhifei Fang 2019-08-30 16:02:46 PDT
Created attachment 377761 [details]
Patch
Comment 2 Zhifei Fang 2019-08-30 16:25:26 PDT
Created attachment 377766 [details]
Patch
Comment 3 Jonathan Bedard 2019-08-30 16:41:13 PDT
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 4 Jonathan Bedard 2019-08-30 16:43:46 PDT
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.
Comment 5 Zhifei Fang 2019-08-30 17:06:17 PDT
Created attachment 377772 [details]
Patch
Comment 6 Zhifei Fang 2019-08-30 17:23:52 PDT
(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 7 WebKit Commit Bot 2019-08-30 21:11:28 PDT
Comment on attachment 377772 [details]
Patch

Clearing flags on attachment: 377772

Committed r249358: <https://trac.webkit.org/changeset/249358>
Comment 8 WebKit Commit Bot 2019-08-30 21:11:29 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2019-08-30 21:12:15 PDT
<rdar://problem/54908092>