Design systems search #405

BeeSeeWhy · 2024-09-05T20:20:47Z

Closes #274

Seach Bar component created

Component Example:

Hover Example:

Focused Example:

Component In Action:

Screen.Recording.2024-09-05.at.1.19.52.PM.mov

thomhickey

Nice work, filters great! Had a few comments and then on Mobile at iPhone 14 Pro Max, the flow kind of falls apart, it's too crowded and I would recommend we style it down in height/font-size for mobile, but maybe the designs didn't call for that.

thomhickey · 2024-09-06T19:11:43Z

src/components/table/table.tsx

@@ -22,6 +21,8 @@ import { SelectableForTable } from "zapatos/schema";
 import emptyState from "../../public/img/empty-state.png";
 import Container from "@mui/material/Container";
 import Image from "next/image";
+import { SearchBar } from "../design_system/searchBar/SearchBar";


curious the relative path imports here? i would tend more toward @components/design_system/searchBar/SearchBar, which then makes table.tsx moveable.

I can definitely change that...I found that usage in another part of the app...but the @ makes more sense

thomhickey · 2024-09-06T19:13:16Z

src/components/table/table.tsx

+            >
+              <SearchBar
+                id="search-input"
+                placeholder="Search"


I think 'Filter' is more applicable to this UX. I know the figma design calls for 'Search', but this is filtering a list, not doing a traditional search, which would return a new list of search results. Just a comment, no big deal.

I didn't build the search functionality, just the UI component. Do you think it would benefit us more to build a traditional search rather than the filtering?

No, I was just saying we should change the placeholder text from 'Search' to 'Filter', but I'm aware that you didn't create that placeholder value in this PR.

Do you think my listing error has to do with my version of node I’m using? It keeps saying prettier is not run, but I’ve even run it separate from the push check and says it’s ok

@BeeSeeWhy do you mean the linting errors above? I added some comments ... I think they are valid for the most part b/c of "unused" rule.

If you're referring to the linting warnings at the bottom of the PR, getting latest from main ought to fix those.

btw if you're having issues w/prettier (as opposed to eslint), try to nuke node_modules and try again. my guess is that there must be something misconfigured in the project b/c a few of us have run into a similar issue.

thomhickey · 2024-09-06T19:14:07Z

src/styles/Input.module.css

-  margin: 2px 4px;
-  padding: 4px 2px;
-}
+/* moved to Form.module.css since this was not styling input, but was styling the form */


maybe remove this comment? it only makes sense if you know what used to be here.

Will do. Thanks.

mrabbitt

@BeeSeeWhy @nickvisut , @chengtchris1 , and I have run into this before where the behavior of prettier on local doesn’t match the GitHub check. What fixed it for us was: Removenode_modules and re-runnpm install. (npm ci would probably work too.) After that, re-run prettier, and it will undo some changes that it introduced incorrectly (mainly trailing commas).

Also, as noted in another comment I think you need to restore the lines you removed from the prettier ignore file.

The styling of the Search component looks ok to me on Desktop. Mobile could be better, as @thomhickey noted.

mrabbitt · 2024-09-06T18:36:04Z

src/backend/db/zapatos/schema.d.ts

Are we sure we want to reformat this file with prettier? It's autogenerated.

mrabbitt · 2024-09-07T15:16:33Z

.prettierignore

-src/backend/db/zapatos
-.next
-.nsm
-.terraform
-*.css.d.ts


I think you need to restore these lines. The lint check is failing with "Code style issues found in 262 files" due to their removal. I'm not sure about some of these rules, but at least the zapatos and css ones are for auto-generated files that shouldn't be manually edited.

Ugh...this what I get for trying to work after getting a COVID vax ... I'll try working on the mobile search as well

nickvisut

left a few comments wrt lint errors and step theming, as well as a Q about prettier

nickvisut · 2024-09-07T22:48:10Z

src/theme.ts

-        }),
-      },
-    },
-    MuiStepLabel: {


hi @BeeSeeWhy, was it intentional to remove the step theming?

nickvisut · 2024-09-12T03:17:33Z

src/components/table/table.tsx

+            >
+              <SearchBar
+                id="search-input"
+                placeholder="Search"


@BeeSeeWhy do you mean the linting errors above? I added some comments ... I think they are valid for the most part b/c of "unused" rule.

If you're referring to the linting warnings at the bottom of the PR, getting latest from main ought to fix those.

btw if you're having issues w/prettier (as opposed to eslint), try to nuke node_modules and try again. my guess is that there must be something misconfigured in the project b/c a few of us have run into a similar issue.

nickvisut · 2024-09-12T03:19:11Z

src/theme.ts

@@ -1,3 +1,4 @@
+import { BorderAllRounded, BorderColor } from "@mui/icons-material";


are these two imports being used?

nickvisut · 2024-09-12T03:20:43Z

package.json

@@ -93,7 +93,7 @@
    "lint-staged": "^13.1.0",
    "nextjs-server-modules": "^4.7.0",
    "nodemailer-mock": "^2.0.1",
-    "prettier": "^2.8.8",
+    "prettier": "2.8.8",


are you having issues w/prettier?

nickvisut · 2024-09-12T04:20:15Z

src/backend/db/zapatos/schema.d.ts

-declare module 'zapatos/schema' {
-
-  import type * as db from 'zapatos/db';
+declare module "zapatos/schema" {


hmm looks like prettier was accidentally run on this file

(could you revert?)

nickvisut · 2024-09-12T04:23:42Z

src/components/design_system/input/Input.tsx

@@ -1 +1,30 @@
-export {};
+import React from "react";


seems like this is unneeded

nickvisut · 2024-09-12T04:25:00Z

src/components/design_system/input/Input.tsx

+import TextField, { TextFieldProps } from "@mui/material/TextField";
+import { styled } from "@mui/material/styles";
+
+export const TextInput = styled(TextField)<TextFieldProps>(({ theme }) => ({


can either use theme colors from theme.ts instead of var(...) below or remove it altogether

nickvisut · 2024-09-12T04:25:59Z

src/components/design_system/searchBar/SearchBar.tsx

+import React from "react";
+import TextField, { TextFieldProps } from "@mui/material/TextField";
+import { styled } from "@mui/material/styles";
+import { Input, InputBase, InputBaseProps } from "@mui/material";


looks like some of these imports are extraneous and can be removed

BeeSeeWhy added 7 commits June 28, 2024 17:48

attempting to get css correct

148b505

Co-authored-by: Yun Whitman <[email protected]>

4c8709f

updates

db9efee

search changes and input changes

a3d9924

merged main

cbfeab6

added adornment styling

906e545

completed search bar component

5530f17

BeeSeeWhy requested a review from nickvisut September 5, 2024 20:20

BeeSeeWhy self-assigned this Sep 5, 2024

BeeSeeWhy added 4 commits September 5, 2024 14:09

made changes

876c788

made changes

7858de1

fixed errors with Form.module.css and removed Input.module.css

f64a55c

ran prettier to fix errors

4fa827d

thomhickey reviewed Sep 6, 2024

View reviewed changes

mrabbitt requested changes Sep 7, 2024

View reviewed changes

update prettier ignore

3294a09

nickvisut requested changes Sep 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Design systems search #405

Design systems search #405

BeeSeeWhy commented Sep 5, 2024

thomhickey left a comment

thomhickey Sep 6, 2024

BeeSeeWhy Sep 6, 2024

thomhickey Sep 6, 2024

BeeSeeWhy Sep 6, 2024

thomhickey Sep 7, 2024

BeeSeeWhy Sep 7, 2024

nickvisut Sep 12, 2024

thomhickey Sep 6, 2024

BeeSeeWhy Sep 6, 2024

mrabbitt left a comment

mrabbitt Sep 6, 2024

mrabbitt Sep 7, 2024

BeeSeeWhy Sep 9, 2024

nickvisut left a comment

nickvisut Sep 7, 2024

nickvisut Sep 12, 2024

nickvisut Sep 12, 2024

nickvisut Sep 12, 2024

nickvisut Sep 12, 2024

nickvisut Sep 12, 2024

nickvisut Sep 12, 2024

nickvisut Sep 12, 2024

nickvisut Sep 12, 2024

		@@ -1,3 +1,4 @@
		import { BorderAllRounded, BorderColor } from "@mui/icons-material";

Design systems search #405

Are you sure you want to change the base?

Design systems search #405

Conversation

BeeSeeWhy commented Sep 5, 2024

thomhickey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrabbitt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickvisut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment