Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Settings modal jumper #981

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

jasonleyser
Copy link
Contributor

@jasonleyser jasonleyser commented Nov 9, 2021

Component for new settings jumper, including the modal structure, sidebar and the first profile tab

  • settings jumper, open/closing
  • sidebar with avatar, username, email and tab buttons
  • profile tab with avatar upload, display name and bio change
  • replaced the settings link in the drop down menu to open the modal

┆Issue is synchronized with this Notion page by Unito

Component for new settings jumper, including the modal structure, sidebar and the first profile tab
@martinalong
Copy link
Collaborator

martinalong commented Nov 10, 2021

image
I think whatever wrapper you put around "settings" erased the styling of the word. Also you should be able to click on any part of the button to open the jumper, not just the word portion of it. Currently it doesn't work if you click to the right of the word

@martinalong
Copy link
Collaborator

image
When the settings jumper opens, it should close the top left popover menu

@martinalong
Copy link
Collaborator

image
Can you make sure the vertical spacing here matches the design? The spacing between each section is different here when they should all be the same

@martinalong
Copy link
Collaborator

image
check the border radius in the design for the images. this border radius is too small

@martinalong
Copy link
Collaborator

image
Border radius on all the buttons and inputs is also different from the designs

@martinalong
Copy link
Collaborator

Let's make sure that user.body has a length limit of 2000, name has a length limit of 255

@martinalong
Copy link
Collaborator

The profile image upload doesn't seem to be working right now btw

@martinalong
Copy link
Collaborator

martinalong commented Nov 10, 2021

image
Left/right padding here looks too wide or something. can you double check it against the designs?

@martinalong
Copy link
Collaborator

Also, we are not going to have the theme tab for now b/c implementing darkmode is pretty involved. So you can just take that out

@martinalong
Copy link
Collaborator

image
Make sure to truncate really long emails that will overflow onto another line with an ellipsis e.g. reallylongemail@gmail....

import { ButtonPrimary, ButtonSecondary } from "~/components/system/components/Buttons";
import * as Type from "~/components/system/components/Typography";

const STYLES_FLEX_ROW = `
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a style in common/styles.js for this called HORIZONTAL_CONTAINER. There's other commonly used styles there. Go check it out and see.

To use it, import it like:
import * as Styles from "~/common/styles";
and use it like this:

You can add on styles to it with inline styles like this:
image

You can also add on styles to it like this, then use the resulting css style:
image

css design updates from the figma file
<div css={STYLES_MENU_ITEM_ICON}>
{tab.icon}
</div>
{tab.title}
Copy link
Collaborator

Choose a reason for hiding this comment

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

make sure every text is wrapped by a style.
I think this one should be: <SYSTEM.P2>{tab.title}</SYSTEM.P2>

size={48}
/>
<div css={STYLES_ACCOUNT_USER}>
<p style={{ fontWeight: "500", fontSize: "14px" }}>{props.viewer.name}</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be <SYSTEM.H5 style={{ marginBottom: "2px"}}>{props.viewer.name}</SYSTEM.H5>

/>
<div css={STYLES_ACCOUNT_USER}>
<p style={{ fontWeight: "500", fontSize: "14px" }}>{props.viewer.name}</p>
<p style={{ fontWeight: "normal", fontSize: "12px" }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this would be <SYSTEM.P3>{truncateString(props.viewer.email, 20)}</SYTEM.P3>

width: 100%;
height: 32px;
border-radius: 12px;
padding: 8px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the padding and alignment here is off, refer to design file for this.

<div css={STYLES_ACCOUNT}>
<ProfilePhoto
user={props.viewer}
style={{ marginBottom: "4px", borderRadius: '10px' }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

borderRadius would be 12px, refer to design file for this.

width: 448px;
height: 50px;
padding: 12px 20px;
background-color: ${Constants.system.white};
Copy link
Collaborator

Choose a reason for hiding this comment

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

${Constants.semantic.bgWhite}

width: 190px;
height: 100%;
padding: 20px 0px;
border-right: 1px solid ${Constants.system.grayLight5};
Copy link
Collaborator

Choose a reason for hiding this comment

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

${Constants.semantic.borderGrayLight}

margin-top: 24px;
`;

const STYLES_ITEM_HEADER = css`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you could use <SYSTEM.H6> rather than a separate style.

<ProfilePhoto
user={state}
size={48}
style={{ borderRadius: '10px' }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

border radius is 12px in design

right: 0;
width: 448px;
height: 50px;
padding: 12px 20px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

padding: 8px 20px;

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.

None yet

3 participants