Details

Reviewers

atul
ashoat

Commits

rCOMM7f0a10aedab9: [web] Fix "See more..." button alignment in `ChatThreadListSeeMoreSidebars`…
rCOMM87c2f874f3f2: Revert "[web] Refactor `classNames` in `threadRow` to take object instead of…
rCOMM17e301d7b856: [web] Fix "See more..." button alignment in `ChatThreadListSeeMoreSidebars`…

Summary

Addressed issue where the "See more..." button wasn't aligned with the rest of the items in the ChatThreadList.

I saw that Ben and Ashoat mentioned the height of the sidebar item changing in the Linear issue. I'll defer to a later diff to refactor the styling of the "See more..." button if the height change of the sidebar affects anything there.

Before:

59d7bb67-9843-40b4-89b2-1741e00f41af.png (238×784 px, 32 KB)

After:

Screen Shot 2022-03-29 at 10.38.27 AM.png (696×530 px, 45 KB)

More context: https://linear.app/comm/issue/ENG-920/see-more-threads-button-doesnt-look-right-on-web

Test Plan

Ensured that thread list looked correct visually. Tested in Chrome/Safari at different breakpoints.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• abosh created this revision.Mar 29 2022, 10:26 AM

Herald added subscribers: • benschac, • karol, • adrian and 2 others. · View Herald TranscriptMar 29 2022, 10:26 AM

• abosh published this revision for review.Mar 29 2022, 10:27 AM

• abosh added a reviewer: ashoat. • abosh added 1 blocking reviewer(s): atul.Mar 29 2022, 10:29 AM

Harbormaster completed remote builds in B7681: Diff 10780.Mar 29 2022, 10:33 AM

• abosh removed a reviewer: ashoat.Mar 29 2022, 10:34 AM

• abosh retitled this revision from [web] Fix "See more..." button alignment in `ChatThreadListSeeMoreSidebars` component to [web] Fix "See more..." button alignment and font size in `ChatThreadListSeeMoreSidebars` component.Mar 29 2022, 10:41 AM

• abosh edited the summary of this revision. (Show Details)

• abosh edited the summary of this revision. (Show Details)Mar 29 2022, 10:56 AM

This revision was not accepted when it landed; it landed in state Needs Review.Mar 29 2022, 11:09 AM

Closed by commit rCOMM17e301d7b856: [web] Fix "See more..." button alignment in `ChatThreadListSeeMoreSidebars`… (authored by • abosh). · Explain Why

This revision was automatically updated to reflect the committed changes.

• abosh added a commit: rCOMM17e301d7b856: [web] Fix "See more..." button alignment in `ChatThreadListSeeMoreSidebars`….

atul added a commit: rCOMM87c2f874f3f2: Revert "[web] Refactor `classNames` in `threadRow` to take object instead of….Mar 29 2022, 11:14 AM

atul added a reverting change: rCOMM204334ee6a07: Revert "[web] Fix "See more..." button alignment in….

• abosh reopened this revision.Mar 29 2022, 2:33 PM

Reopening diff

Harbormaster completed remote builds in B7690: Diff 10792.Mar 29 2022, 2:40 PM

atul requested changes to this revision.Mar 29 2022, 3:02 PM

atul added inline comments.

web/chat/chat-thread-list.css
155 ↗	(On Diff #10792)	Let's skip this for now. From Linear (https://linear.app/comm/issue/ENG-920#comment-cdd44cbc): We simply need to make the "See more…" match the design on the sidebar item, in all ways (alignment, height, padding, font size, etc.).

This revision now requires changes to proceed.Mar 29 2022, 3:02 PM

atul added a reviewer: ashoat.Mar 29 2022, 3:02 PM

Removed font-size change.

atul accepted this revision.Mar 29 2022, 3:07 PM

atul retitled this revision from [web] Fix "See more..." button alignment and font size in `ChatThreadListSeeMoreSidebars` component to [web] Fix "See more..." button alignment in `ChatThreadListSeeMoreSidebars` component.

atul edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B7692: Diff 10794.Mar 29 2022, 3:13 PM

In the screenshots it doesn't look like everything is the same between the "See more" item and the sidebar items. Is the font size the same? Is the spacing between items (padding / margin) really the same?
I mentioned in the Linear task that Ben has a diff out that changes the height of sidebar items. We should make sure that we're building on top of that diff here... the intention is for the "See more" item to match the sidebar items, and this won't be true anymore if the sidebar items change
- You can use arc patch Dsomething to pull in Ben's diff to your local environment
- We should specify a dependency between this diff and that one on Phabricator ("Edit Related Revisions" or add "Depends on Dsomething" to the diff description)
- We should make sure the screenshot etc. are based on top of Ben's diff