Page MenuHomePhabricator

[web] Refactor search styling to match Figma
ClosedPublic

Authored by jacek on Mar 8 2022, 4:06 AM.
Tags
None
Referenced Files
F3486245: D3368.diff
Wed, Dec 18, 4:03 AM
Unknown Object (File)
Nov 16 2024, 1:51 AM
Unknown Object (File)
Nov 5 2024, 9:20 PM
Unknown Object (File)
Nov 3 2024, 7:47 AM
Unknown Object (File)
Oct 16 2024, 8:27 AM
Unknown Object (File)
Oct 14 2024, 2:19 AM
Unknown Object (File)
Oct 14 2024, 2:19 AM
Unknown Object (File)
Oct 14 2024, 2:19 AM

Details

Summary

Styling changes for Search making the component similar to the one from Figma in Members modal.

Figma (in members modal):

Screenshot_Google Chrome_2022-03-11_120152.png (274×1 px, 19 KB)

Implementation (now only in thread search)
Before:
Screenshot_Google Chrome_2022-03-08_131939.png (196×936 px, 16 KB)

After
v2
Screenshot_Google Chrome_2022-03-11_120102.png (138×800 px, 10 KB)

Test Plan

The thread search bar should now be similar to the search in modal design.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

jacek added reviewers: benschac, atul.
jacek edited the summary of this revision. (Show Details)
benschac added inline comments.
web/components/search.css
16 ↗(On Diff #10137)

in web/style.css

body {
  font-family: var(--font-stack);
  background: var(--bg);
  height: 100%;
  overflow: hidden;
  font-size: 1.6rem;
}

base body font is 1.6rem which should be a variable. Either way I don't think we need to define font-size here since body by default is 1.6rem.

17 ↗(On Diff #10137)

we have a defined line-height in typography: --line-height-text: 1.5;

web/components/search.react.js
33 ↗(On Diff #10137)

could we create a css selector:

.searchIcon svg {
 color: var(--search-icon-color);
}

and --search-icon-color is --shades-black-60.

ideally, trying to avoid hard coding colors.

34 ↗(On Diff #10137)

Could you leave a TODO: update icon size comment here. The diff with updated paddings should land shortly.

This revision now requires changes to proceed.Mar 8 2022, 11:11 AM
web/components/search.css
16 ↗(On Diff #10137)

It is overwritten by browser stylesheets for input HTML element. When font size is not set here, its size is 13.33px.
The same is for line-height - it is normal by default, which is ~1.2

Maybe we should consider resetting the default browser styles for some elements (bi think button has the same issue), so we don't have to overwrite it?

17 ↗(On Diff #10137)

See comment above ^

Rebase & adjust icon styling

Improved clear search button to also match Figma.

benschac added inline comments.
web/components/search.css
38 ↗(On Diff #10287)

can we avoid using margin? and just add padding to the parent element?

16 ↗(On Diff #10137)
This revision is now accepted and ready to land.Mar 11 2022, 7:35 AM

Replaced margin with padding

This revision now requires review to proceed.Mar 25 2022, 4:43 AM
This revision is now accepted and ready to land.Mar 25 2022, 7:31 AM