Page MenuHomePhabricator
Differential D4061

[lib, web] [feat] [ENG-536] hook up sidebar data to search modal
AbandonedPublic
Actions

Authored by tomek on May 16 2022, 2:31 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 29, 10:03 AM
Unknown Object (File)
Wed, Nov 27, 7:42 PM
Unknown Object (File)
Wed, Nov 20, 11:59 PM
Unknown Object (File)
Wed, Nov 20, 5:00 PM
Unknown Object (File)
Wed, Nov 20, 5:00 PM
Unknown Object (File)
Wed, Nov 20, 5:00 PM
Unknown Object (File)
Wed, Nov 20, 5:00 PM
Unknown Object (File)
Sat, Nov 16, 12:33 AM
View All Files
Subscribers
• abosh
• adrian
ashoat

Details

Reviewers
atul
• jacek
• benschac
Summary

Add search sidebar functionality to search modal

I still need to style the sidebar items and the modal when it doesn't have content which I'll update in a following diff.

web:

CleanShot 2022-05-16 at 18.00.57.gif (462×800 px, 831 KB)

native:
CleanShot 2022-05-16 at 18.00.57.gif (462×800 px, 831 KB)

Test Plan

Go to sidebar in web/ and native. Test functionality. Both should work as they did before. Web UI should be updated

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• benschac created this revision.May 16 2022, 2:31 PM
Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptMay 16 2022, 2:31 PM
• benschac edited the summary of this revision. (Show Details)May 16 2022, 2:33 PM
• benschac added inline comments.
lib/hooks/search-sidebars.js
71 ↗(On Diff #12776)

clear query isn't used now that we're using <SearchModal>

Harbormaster completed remote builds in B9185: Diff 12776.May 16 2022, 2:37 PM
• benschac requested review of this revision.May 16 2022, 2:37 PM
• benschac added a parent revision: D4060: [native, lib, web] [refactor] move text out of state.May 16 2022, 2:38 PM
• benschac edited the summary of this revision. (Show Details)May 16 2022, 3:03 PM
• benschac updated this revision to Diff 12819.May 17 2022, 7:57 AM

rebase

Harbormaster failed remote builds in B9226: Diff 12819!May 17 2022, 8:02 AM
• benschac updated this revision to Diff 12827.May 17 2022, 8:02 AM

rebase

Harbormaster completed remote builds in B9234: Diff 12827.May 17 2022, 8:39 AM
• benschac added a child revision: D4066: [web] [refactor] [ENG-536] remove old sidebar styling from side bar modal.May 17 2022, 11:07 AM
tomek requested changes to this revision.May 19 2022, 1:25 AM

After this diff, something is wrong with the design of the hook. We have two ways of updating the search text: by calling the hook with different text and by calling onChangeSearchText. Could you spend some time searching for a design which is easier to reason about? If there's none, please try to explain why do we need to have these complicated state transitions.

This revision now requires changes to proceed.May 19 2022, 1:25 AM
• benschac updated this revision to Diff 13092.May 24 2022, 10:24 AM

rebase

• benschac planned changes to this revision.May 24 2022, 10:25 AM
Harbormaster completed remote builds in B9454: Diff 13092.May 24 2022, 10:49 AM
tomek commandeered this revision.Apr 3 2024, 2:35 AM
tomek abandoned this revision.
tomek edited reviewers, added: • benschac; removed: tomek.

The task from the title https://linear.app/comm/issue/ENG-536/sidebar-list-modal-re-style - is marked as done. If we decide to do something more about that issue, it would be probably easier to do it from scratch instead of figuring out how to make this stack solve the problem.

Revision Contents
Changeset List

PathSize
lib/
hooks/
22 lines
web/
modals/
chat/
106 lines

Diff 13092

View Options

lib/hooks/search-sidebars.js

Loading...
View Options

web/modals/chat/sidebar-list-modal.react.js

Loading...