Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

refactor: Update users page to looks like others#1850

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
BrunoQuaresma merged 4 commits intomainfrombq/update-user-page
May 27, 2022

Conversation

BrunoQuaresma
Copy link
Collaborator

@BrunoQuaresmaBrunoQuaresma commentedMay 27, 2022
edited
Loading

  • Remove the "Users" header
  • Remove the old header component because it is not used anymore. We probably want to create a new one.
  • Extract the "Avatar data" logic into theAvatarData component so we can keep it consistent between templates, workspaces, and users' views.

Screen Shot 2022-05-27 at 13 10 20

@BrunoQuaresmaBrunoQuaresma requested a review froma team as acode ownerMay 27, 2022 16:10
@BrunoQuaresmaBrunoQuaresma self-assigned thisMay 27, 2022
link?:string
}

exportconstAvatarData:React.FC<AvatarDataProps>=({ title, subtitle, link})=>{
Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

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

I didn't find a better name for it 😓

Copy link
Member

Choose a reason for hiding this comment

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

Seems fine!

BrunoQuaresma reacted with thumbs up emoji
"& .MuiSelect-root":{
// Adjusting padding because it does not have label
paddingTop:theme.spacing(1.5),
paddingBottom:theme.spacing(1.5),
Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

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

Added these because the select when there is no label is very BIG.

}

constuseStyles=makeStyles((theme)=>({
actions:{
Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

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

This is probably something we want to extract as well.

link?:string
}

exportconstAvatarData:React.FC<AvatarDataProps>=({ title, subtitle, link})=>{
Copy link
Member

Choose a reason for hiding this comment

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

Seems fine!

BrunoQuaresma reacted with thumbs up emoji
expect(rows).toHaveLength(MockBuilds.length)

// Wait for the results to be loaded
awaitwaitFor(async()=>{
Copy link
CollaboratorAuthor

@BrunoQuaresmaBrunoQuaresmaMay 27, 2022
edited
Loading

Choose a reason for hiding this comment

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

Not sure why, but this test should had fail before since it was "wrong"

@BrunoQuaresmaBrunoQuaresmaenabled auto-merge (squash)May 27, 2022 16:37
@BrunoQuaresmaBrunoQuaresma merged commit7eacab8 intomainMay 27, 2022
@BrunoQuaresmaBrunoQuaresma deleted the bq/update-user-page branchMay 27, 2022 16:47
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@kylecarbskylecarbskylecarbs approved these changes

Assignees

@BrunoQuaresmaBrunoQuaresma

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@BrunoQuaresma@kylecarbs

[8]ページ先頭

©2009-2025 Movatter.jp