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

Edit profile #384

Merged
merged 7 commits into from
Jan 29, 2020
Merged

Edit profile #384

merged 7 commits into from
Jan 29, 2020

Conversation

quininez
Copy link
Member

@quininez quininez commented Jan 28, 2020

Edit profile and refactor file structure for design system

Description

  • profile card edit functionality: can choose between different providers of profile data using a popover
  • avatar placeholder loading properly, will use a single placeholder for visitors
  • file structure for design system: group components and styled elements files in folders
  • theme loader component: simple wrapper component over Grommet, providing current theme

Checklist

  • I have read the README document
  • I have read the CONTRIBUTING document
  • I have added tests to cover my changes
  • All new and existing tests passed
  • My commit message follows Conventional Commit Guideline

@quininez quininez marked this pull request as ready for review January 29, 2020 08:20
avatarPopoverOpen &&
profileProvidersData &&
profileProvidersData.avatarProviders &&
profileProvidersData.avatarProviders.length !== 0 && (
Copy link
Member

Choose a reason for hiding this comment

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

what is the expected behaviour if one of these conditions are not met? (especially the profileProvidersData)
the user will think that the button doesn't work

Copy link
Member Author

Choose a reason for hiding this comment

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

true, i should also hide the button when we dont have that data

import colors from '../colors-light';
import shapes from '../shapes';
import createGrommetTheme from './index';

export function createTheme(overrides?: object): DefaultTheme {
export function createTheme(overrides?: object): any {
Copy link
Member

Choose a reason for hiding this comment

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

Do you have ts issues when DefaultTheme type is used?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, grommet introduced a type for the grommet theme, but this slipped, i should update it with an interface that extends from that grommet theme interface

@quininez quininez requested a review from SeverS January 29, 2020 11:56
const StyledAvatarEditDiv = styled.div`
border-radius: 50%;
position: relative;
left: 56px;
Copy link
Member

Choose a reason for hiding this comment

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

this file has more px values

cursor: pointer;
}
border-bottom: 1px solid ${props.theme.colors.border}
height: 49px;
Copy link
Member

Choose a reason for hiding this comment

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

px -> em

@quininez quininez requested a review from SeverS January 29, 2020 12:59
@quininez quininez merged commit 8dbe6a8 into master Jan 29, 2020
@quininez quininez deleted the edit-profile branch January 29, 2020 13:03
josenriagu added a commit that referenced this pull request Sep 11, 2020
* feat(): edit profile WIP

* fix(): avatar fix and refactor file structure

* fix(): ref types, file paths

* fix():  file paths

* refactor(): split profile card editable fields into multiple controlled components

* fix(): theme type, hide edit buttons when missing data, replace px with em

* fix(): replace rest of px with em
kenshyx pushed a commit that referenced this pull request Nov 30, 2021
* feat(): edit profile WIP

* fix(): avatar fix and refactor file structure

* fix(): ref types, file paths

* fix():  file paths

* refactor(): split profile card editable fields into multiple controlled components

* fix(): theme type, hide edit buttons when missing data, replace px with em

* fix(): replace rest of px with em
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants