Bug 223215 - [resultsdbpy] Allow user to change commit representation
Summary: [resultsdbpy] Allow user to change commit representation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-15 14:16 PDT by Jonathan Bedard
Modified: 2021-03-16 15:10 PDT (History)
4 users (show)

See Also:


Attachments
Patch (17.99 KB, patch)
2021-03-15 14:27 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (17.97 KB, patch)
2021-03-16 13:35 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch for landing (19.15 KB, patch)
2021-03-16 14:00 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2021-03-15 14:16:52 PDT
Allow users to pick how commits are represented for a given repository.
Comment 1 Radar WebKit Bug Importer 2021-03-15 14:17:16 PDT
<rdar://problem/75446602>
Comment 2 Jonathan Bedard 2021-03-15 14:27:18 PDT
Created attachment 423240 [details]
Patch
Comment 3 dewei_zhu 2021-03-16 10:55:59 PDT
Comment on attachment 423240 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423240&action=review

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/commit.js:150
> +        let preference = CommitBank._representationCache ? CommitBank._representationCache[this.repository_id].representation : null;

Use `const` since we are not modifying?

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/drawer.js:390
> +                    });
> +                }
> +            }
> +        });

Mismatched indentation.
Comment 4 Zhifei Fang 2021-03-16 11:09:48 PDT
Comment on attachment 423240 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423240&action=review

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/drawer.js:379
> +            element.onclick = () => {

We should use onStateUpdate for rendering isExpanded, without it, your update will be outside requestAnimationFrame, we should make sure all of our DOM updates inside of it.

You can do:
const expander = REF.createRef({...});
expander.fromEvent('click').action(() => expander.setState({isExpanded: !isExpanded}));
Comment 5 dewei_zhu 2021-03-16 12:57:03 PDT
Comment on attachment 423240 [details]
Patch

r=me
Comment 6 Jonathan Bedard 2021-03-16 13:35:51 PDT
Created attachment 423392 [details]
Patch
Comment 7 Jonathan Bedard 2021-03-16 14:00:51 PDT
Created attachment 423396 [details]
Patch for landing
Comment 8 EWS 2021-03-16 15:10:00 PDT
Committed r274523: <https://commits.webkit.org/r274523>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423396 [details].