move sidebar search to lib/hooks from web,native
Details
Details
use feature in both web and native
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- side-bar-list-modal-ENG-536
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 11–12 ↗ | (On Diff #12598) | Missing $ReadOnly :( |
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 10–13 ↗ | (On Diff #12653) | Think we can do something like this? |
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 10–13 ↗ | (On Diff #12653) | that looks chill. Going to try it. |
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 11–12 | Missing $ReadOnly... again :( :( | |
Comment Actions
Requesting review so this diff gets back in @ashoat's queue. I have a question about readonly syntax.
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 11–12 | I don't understand why this isn't valid. We're using a $ReadonlySet. If you look at the previous diff changes, I had a ReadOnly, but @atul made that suggestion to use $ReadOnlySet which seems like it's just a convenience thing. My understanding is:  | |
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 11–12 | I think what's missing is the plus signs in front of text and results? | |
Comment Actions
Very concerning that @benschac is confused about the difference between making a property of an object read-only, versus making the value stored in that property read-only. These are different concepts and it's critical that everybody on the team understands the difference.
My time is really limited, so would appreciate if @benschac could try to learn about the difference here without requiring my involvement. @benschac, if this is still confusing can you try to maybe consult @atul to learn more about what's going on here?
Additionally, there are still objects here that are not $ReadOnly!! I don't know how this is confusing... literally just look at every single object, and make sure each property has a plus sign. Please don't pass this back to me until this is addressed... it's been months of me repeating this feedback over and over, and it gets 10x more frustrating for me each time I have to repeat it.
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 18–20 ↗ | (On Diff #12746) | Missing $ReadOnly... again... again :( :( :( |
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 31 ↗ | (On Diff #12821) | listData gets renamed to sidebars in this diff: https://phabricator.ashoat.com/D4067 |