Conversation
Nutomic
requested review from
SleeplessOne1917,
dessalines and
matc-pub
as code owners
dessalines
left a comment
dessalines
left a comment
There was a problem hiding this comment.
Something weird going on with the diff, this really only needs to add that button for mobile, as well as {this.showMobileExpanded && filter.... around the ones you want to hide / show.
There was a problem hiding this comment.
Create a variant or prop to the expand chip checkbox that uses these icons. Because your expand button should look similar to the other already existing ones.
src/shared/components/home/home.tsx
Outdated
| <button | ||
| className="col btn btn-ghost" | ||
| onclick={_ => handleHideSelectButtons(this)} | ||
| > |
|
The expand should also be used on desktop. Reason is that these options ("show hidden posts" and "hide read posts") are rarely used, and shouldnt distract the user from more important actions like changing the sort type. I dont want to use ExpandChipCheckbox because its already intuitive with the arrow icon, and doesnt need any text. And btn-ghost doesnt catch so much attention, it shouldnt have any background or border. |
| return ( | ||
| <div className="g-3 mb-3"> | ||
| <div className="row row-cols-auto align-items-center"> | ||
| <div> |
There was a problem hiding this comment.
This extra wrapper div is probably not necessary.
|
I tested and this isn't working at all for some reason, neither on mobile nor desktop views. The diff is still strange, it should only need to add |
| ? "d-none" | ||
| : "row row-cols-auto mt-2" |
There was a problem hiding this comment.
Here's the culprit. Don't do this, just surround the specific filters you want to hide with {!this.state.selectButtonsHidden && <div
Updated with changes from #3962:
I tried to keep the expanded buttons in the same row but it looks strange: 
|
2 participants
These are rarely used and shouldnt take up space unnecessarily.
collapsed:
expanded:
mobile expanded:
Same change also needs to be made in community.tsx.