Page MenuHomePhabricator

[web] Refactor `ChatTabs` to use `Tabs` component
ClosedPublic

Authored by jacek on Mar 9 2022, 3:20 AM.
Tags
None
Referenced Files
F3486398: D3373.diff
Wed, Dec 18, 4:50 AM
Unknown Object (File)
Wed, Nov 27, 4:28 AM
Unknown Object (File)
Nov 7 2024, 9:16 AM
Unknown Object (File)
Nov 5 2024, 9:41 PM
Unknown Object (File)
Nov 2 2024, 5:52 PM
Unknown Object (File)
Nov 2 2024, 3:34 PM
Unknown Object (File)
Oct 24 2024, 5:29 AM
Unknown Object (File)
Oct 24 2024, 2:17 AM

Details

Summary

Used introduced Tabs component in ChatTabs, replacing existing solution. Chat borders design now matches Figma design and elements have onHover effect. The behavior of panel should remain the same.

Before:

Screenshot_Google Chrome_2022-03-09_122729.png (152×834 px, 14 KB)

After:

Screenshot_Google Chrome_2022-03-09_122708.png (164×844 px, 14 KB)

Test Plan

Run web app and try to switch Between Focus and Background tabs. It should work as before.

Diff Detail

Repository
rCOMM Comm
Branch
jacek/chat-tabs
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek requested changes to this revision.Mar 10 2022, 2:50 AM

Just an idea: we can consider using children instead of a prop with an array. Initially I thought it might be complicated, but I used this approach in D3341 and it wasn't that hard

web/chat/chat-tabs.react.js
29–33

Can we use more concrete type, e.g. 'Background' | 'Focus' and avoid using invariants?

66
This revision now requires changes to proceed.Mar 10 2022, 2:50 AM

Used Tabs.Container and Tabs.Item

tomek added a reviewer: ashoat.
tomek added inline comments.
web/chat/chat-tabs.react.js
29 ↗(On Diff #10303)

Just to make sure, as it looks really interesting. So does that mean that if we try to use a tab with a different id a flow error would be returned?

web/chat/chat-tabs.react.js
29 ↗(On Diff #10303)

Yes, if we try to execute this callback with other id, flow error is thrown.

Makes sense to me! The Tabs.Item header prop probably could've been a Tabs.Header component, but it doesn't matter that much

This revision is now accepted and ready to land.Mar 14 2022, 9:21 PM