Web/feature/user overview v2 by iamseye · Pull Request #164 · Couchers-org/web-frontend

iamseye · 2022-07-12T20:25:20Z

move userOverview components to the component folder
add HTML symbol & update v2 UI of userOverview
show user Id

Noted, left some questions for design. still looks very different from the design: https://www.figma.com/file/z1aQUklCQewLsEFUo2HsLz/Desktop-Mockups?node-id=9722%3A63431

Before	After

Web frontend checklist

Formatted my code with make format
There are no warnings from make lint
There are no console warnings when running the app
Added any new components to storybook
Added tests where relevant
All tests pass
Clicked around my changes running locally and it works
Checked Desktop, Mobile and Tablet screen sizes

* master: Translation updates (#170) Fixed some miswording in the handbook (#168) Removed a redundant word (#166) Web/chore/profile i18n keys (#138) # Conflicts: # components/UserOverview/UserOverview.test.tsx # features/profile/view/Overview.tsx # features/profile/view/leaveReference/LeaveReferencePage.tsx

lucaslcode

This looks great! Sorry it took so long to review!
Just some minor nits that are not particularly important.

lucaslcode · 2022-07-28T09:43:36Z

    justifyContent: "center",
    alignItems: "stretch",
-    padding: theme.spacing(0.5),
+    padding: "0px",


Suggested change

padding: "0px",

padding: 0,

lucaslcode · 2022-07-28T09:43:55Z

+    },
+    "& > button": {
+      fontWeight: "bold",
+      padding: theme.spacing(1.6),


Why 1.6? If it's to pixel-match figma I'd probably leave it as 1.5 for consistency (we tend to try to have all uses of spacing in 0.5 increments)

lucaslcode · 2022-07-28T09:47:34Z

-        </Typography>
-        <Typography variant="body1" className={classes.intro}>
-          {user.city}
+        <Typography variant="h1" className={classes.intro} align={"left"}>


Suggested change

<Typography variant="h1" className={classes.intro} align={"left"}>

<Typography variant="h1" className={classes.intro} align="left">

lucaslcode · 2022-07-28T09:47:49Z

+          {name}
        </Typography>
+        <Typography
+          color={"textSecondary"}


Suggested change

color={"textSecondary"}

color="textSecondary"

lucaslcode · 2022-07-28T09:48:17Z

-      {actions && (
-        <CardActions className={classes.cardActions}>{actions}</CardActions>
-      )}
+      <Typography color={"primary"} variant="body1" className={classes.intro}>


Suggested change

<Typography color={"primary"} variant="body1" className={classes.intro}>

<Typography color="primary" variant="body1" className={classes.intro}>

lucaslcode · 2022-07-28T09:49:24Z

@@ -1,10 +1,10 @@
 import Alert from "components/Alert";
 import Button from "components/Button";
+import UserOverview from "components/UserOverview/UserOverview";


Suggested change

import UserOverview from "components/UserOverview/UserOverview";

import UserOverview from "components/UserOverview";

Since you added the index file

lucaslcode · 2022-07-28T09:49:32Z

 import Hidden from "@material-ui/core/Hidden";
 import Alert from "components/Alert";
 import CircularProgress from "components/CircularProgress";
+import UserOverview from "components/UserOverview/UserOverview";


Suggested change

import UserOverview from "components/UserOverview/UserOverview";

import UserOverview from "components/UserOverview";

darrenvong

This is probably more of a design question, but for users with super long names (as we have deliberately set in the test account to see how it looks for some other feature...), do we want to center the name instead of having it left aligned like it does now?

darrenvong · 2022-07-31T13:51:00Z

 import users from "test/fixtures/users.json";

-import { ProfileUserProvider } from "../hooks/useProfileUser";
+import { ProfileUserProvider } from "../../features/profile/hooks/useProfileUser";


We should prefer absolute imports here when it's more than one level of . or .. to ease future changes if needed rather than having to guess how many .. levels to go up.

darrenvong · 2022-07-31T13:51:30Z

+import {
+  hostingStatusLabels,
+  meetupStatusLabels,
+} from "../../features/profile/constants";
+import { ProfileUserProvider } from "../../features/profile/hooks/useProfileUser";


Same comment on absolute import as before

darrenvong · 2022-07-31T13:51:36Z

+import { useProfileUser } from "../../features/profile/hooks/useProfileUser";
+import {
+  ReferencesLastActiveLabels,
+  ResponseRateLabel,
+} from "../../features/profile/view/userLabels";


Same comment on absolute import as before

darrenvong · 2022-10-30T23:05:19Z

@iamseye How are you getting on with this? Will you have time to wrap this up?

iamseye added 3 commits July 12, 2022 15:40

chore(frontend): move userOverview to components

fe67622

feat(frontend): added @ symbol, adjusted UI

657361d

feat(frontend): added divider

6e9ae52

iamseye temporarily deployed to web-frontend-web-feature-user-overview-v2 July 12, 2022 20:44 Inactive

iamseye temporarily deployed to web-frontend-web-feature-user-overview-v2 July 22, 2022 15:55 Inactive

chore(frontend): adjust UI

513a261

iamseye temporarily deployed to web-frontend-web-feature-user-overview-v2 July 22, 2022 19:09 Inactive

iamseye marked this pull request as ready for review July 22, 2022 19:12

iamseye self-assigned this Jul 22, 2022

iamseye requested a review from lucaslcode July 22, 2022 19:12

chore(frontend): add test

6fcbb00

iamseye deployed to web-frontend-web-feature-user-overview-v2 July 22, 2022 19:34 View deployment

lucaslcode suggested changes Jul 28, 2022

View reviewed changes

darrenvong reviewed Jul 31, 2022

View reviewed changes

aapeliv added the web label Apr 20, 2024

	<Typography variant="h1" className={classes.intro} align={"left"}>
	<Typography variant="h1" className={classes.intro} align="left">

	<Typography color={"primary"} variant="body1" className={classes.intro}>
	<Typography color="primary" variant="body1" className={classes.intro}>

	import UserOverview from "components/UserOverview/UserOverview";
	import UserOverview from "components/UserOverview";

Uh oh!

Conversation

iamseye commented Jul 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucaslcode left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darrenvong left a comment

Choose a reason for hiding this comment

Uh oh!

darrenvong Jul 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darrenvong commented Oct 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iamseye commented Jul 12, 2022 •

edited

Loading

darrenvong Jul 31, 2022 •

edited

Loading