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.

Sign up for GitHub

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

Jump to bottom

Implement Person Page #4

Merged
merged 11 commits into from
Dec 14, 2023
Merged

Implement Person Page #4

merged 11 commits into from
Dec 14, 2023

Conversation

Pdzly
Copy link
Member

@Pdzly Pdzly commented Dec 12, 2023

No description provided.

@Pdzly Pdzly requested a review from kgilles December 12, 2023 14:37
@Pdzly Pdzly self-assigned this Dec 12, 2023
@Pdzly Pdzly changed the title Added Some Implement Person Page Dec 12, 2023
kgilles
kgilles requested changes Dec 12, 2023
View reviewed changes
Copy link
Member

@kgilles kgilles left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great progress!

Perhaps you'd already planned for this but make sure the page and components look good in mobile and other generally smaller screens.

There's also some dark text on dark background when using dark mode that needs updated.

src/app/user/page.tsx Outdated Show resolved Hide resolved
src/app/user/page.tsx Outdated Show resolved Hide resolved
src/components/moderates-list/index.tsx Outdated Show resolved Hide resolved
src/components/moderates-list/index.tsx Outdated Show resolved Hide resolved
src/components/moderates-list/index.tsx Outdated Show resolved Hide resolved
src/components/person-detail/index.tsx Show resolved Hide resolved
src/components/person-detail/index.tsx Show resolved Hide resolved
src/components/person-detail/index.tsx Outdated Show resolved Hide resolved
kgilles
kgilles requested changes Dec 13, 2023
View reviewed changes
Copy link
Member

@kgilles kgilles left a comment

Choose a reason for hiding this comment

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

Mobile view is quite crowded horizontally with the moderates list. I'd recommend giving the user page a more column-like flow of components. Since the horizontal viewport is usually narrow in smaller devices.

image

src/app/user/page.tsx Outdated Show resolved Hide resolved
src/utils/TailwindMaterial.ts Outdated Show resolved Hide resolved
tailwind.config.ts Outdated Show resolved Hide resolved
kgilles
kgilles requested changes Dec 14, 2023
View reviewed changes
Copy link
Member

@kgilles kgilles left a comment

Choose a reason for hiding this comment

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

Just one or two more minor things. Patch those up, move this PR out of draft mode and I'll approve things when I wake up tomorrow.

src/components/person-comments-posts/index.tsx Outdated Show resolved Hide resolved
kgilles
kgilles requested changes Dec 14, 2023
View reviewed changes
src/app/user/[userslug]/page.tsx Outdated Show resolved Hide resolved
src/components/post-header/index.tsx Outdated Show resolved Hide resolved
@Pdzly Pdzly marked this pull request as ready for review December 14, 2023 18:24
@Pdzly Pdzly enabled auto-merge (squash) December 14, 2023 18:28
@Pdzly Pdzly merged commit 66a887a into main Dec 14, 2023
1 check passed
@Pdzly Pdzly deleted the feature/3-person-page branch December 14, 2023 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kgilles kgilles kgilles approved these changes

Assignees

@Pdzly Pdzly

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Pdzly @kgilles