Page MenuHomePhabricator
Differential D3538

[web] [refactor] destructure props to match project
ClosedPublic
Actions

Authored by • benschac on Mar 29 2022, 7:13 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 15, 9:02 AM
Unknown Object (File)
Tue, Nov 5, 5:39 AM
Unknown Object (File)
Tue, Nov 5, 5:39 AM
Unknown Object (File)
Tue, Nov 5, 5:38 AM
Unknown Object (File)
Tue, Nov 5, 5:35 AM
Unknown Object (File)
Thu, Oct 24, 10:43 PM
Unknown Object (File)
Oct 9 2024, 6:31 PM
Unknown Object (File)
Sep 11 2024, 6:23 AM
View All Files
Subscribers
• abosh
• adrian
ashoat
• karol
tomek

Details

Reviewers
atul
tomek
• jacek
• abosh
Commits
rCOMMe84e0e6f1daa: [web] [refactor] destructure props to match project
Summary

update props to stay consistent with project in side-bar item

Test Plan

side bar item still renders and shows unread as expected

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• benschac created this revision.Mar 29 2022, 7:13 AM
Herald added subscribers: • abosh, • karol, • adrian and 2 others. · View Herald TranscriptMar 29 2022, 7:13 AM
• benschac added a parent revision: D3537: [web] [fix] ENG-761 add padding between timestamp and message.Mar 29 2022, 7:14 AM
• benschac edited parent revisions, added: D3480: [web] [fix] thread list size and paddings are incorrect; removed: D3537: [web] [fix] ENG-761 add padding between timestamp and message.
• benschac added reviewers: tomek, • jacek, • abosh.
Harbormaster completed remote builds in B7675: Diff 10771.Mar 29 2022, 7:20 AM
• benschac requested review of this revision.Mar 29 2022, 7:20 AM
• benschac added a child revision: D3539: [web] [feat] [ENG-766] toggle longer arrow for multiple thread sidebars in chat thread list.Mar 29 2022, 8:34 AM
tomek accepted this revision.Mar 29 2022, 8:51 AM

I'd prefer the former approach - I find it more readable. And I don't think that having this so consistent has a significant impact on the readability and maintainability (even when in all the components we used the same approach when destructuring the props, it wouldn't make the codebase significantly more readable).

Accepting, but I don't think that it matters too much

This revision is now accepted and ready to land.Mar 29 2022, 8:51 AM
• benschac added a child revision: D3568: [web] [fix] match last message in chat thread item to figma design.Mar 30 2022, 10:51 AM
ashoat added a comment.EditedMar 30 2022, 9:56 PM

I also generally don't see the point of diffs like this, and find the former approach slightly more readable anyways

Closed by commit rCOMMe84e0e6f1daa: [web] [refactor] destructure props to match project (authored by • benschac). · Explain WhyMar 31 2022, 7:29 AM
This revision was automatically updated to reflect the committed changes.
• benschac added a commit: rCOMMe84e0e6f1daa: [web] [refactor] destructure props to match project.

Revision Contents
Changeset List

PathSize
web/
chat/
9 lines

Diff 10896

View Options

web/chat/sidebar-item.react.js

Loading...