Skip to content

✨ 40 - user profile page - #46

Merged
anncatton merged 5 commits into
developfrom
40-user-profile-page
Jan 20, 2021
Merged

anncatton merged 5 commits into
developfrom
40-user-profile-page

Conversation

@anncatton

Copy link
Copy Markdown
Contributor

Add route, page component for User.

  • no data connection, just uses a sample User object and apiToken.
  • does not include connecting user to header

Comment thread components/pages/user/index.tsx Outdated

const User = () => {
return (
<PageLayout backgroundColor={theme.colors.white}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about using styled here?

Adding a className prop to PageLayout and then here using styled('PageLayout')background: ${theme.colors.white}`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've run into situations where I want to pass through one style... then it's another css prop... then another... and suddenly the props are filled with css styles which isn't ideal

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah i see what you mean - good idea!

Comment thread components/Link.tsx Outdated
import defaultTheme from './theme';

const StyledLink = styled('a')`
color: ${({ theme }: { theme: typeof defaultTheme }) => css(theme.colors.secondary_accessible)};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general (because we have a very similar setup on platform-ui) do we need to type the theme prop every time?

@anncatton anncatton Jan 19, 2021 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean like this? pardon the wonky formatting
const StyledLink = styled('a')
${({ theme }: { theme: typeof defaultTheme }) => csscolor: ${theme.colors.secondary_accessible}; ${theme.typography.regular}; line-height: 24px; &:hover { color: ${theme.colors.accent}; }}
;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just curious if we need to do the typing every time: ${({theme})} : {theme: typeof defaultTheme

but I see your point.

we've done something like this in a couple of places:

const Ul2 = styled('ul')`
  ${({ theme }) => css(theme.typography.paragraph)};
  padding-left: 0px;
  margin-top: 5px;
  margin-bottom: 30px;
`;

so i guess one can do:

const StyledLink = styled('a')`
${theme} => css`
color: theme.colors.secodary;
&:hover {
color: theme.colors.accent
}`
`

Still trying to figure out the cleanest way to use Emotion!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My editor complains when i don't type it...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

frig. ok not a big deal. just im sure its annoying typing it every single time :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah...i'll keep this in the back of my mind, maybe a nice solution will make itself known :)

@@ -0,0 +1,453 @@
import { css, Global } from '@emotion/core';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This component looks fantastic

Comment thread components/pages/user/index.tsx Outdated
};

const getDayValue = (exp: number) => {
// round or floor?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

off the top of my head I want to say floor because round might be rounding up when a token is actually expired

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point! will change

@anncatton
anncatton merged commit 1fbd5a6 into develop Jan 20, 2021
@anncatton
anncatton deleted the 40-user-profile-page branch January 20, 2021 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants