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

fix(theme-default): Support centering members in VPTeamPage #3796

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

Conversation

stevending1st
Copy link

@stevending1st stevending1st commented Apr 17, 2024

Todo

  • Remove team-test.md before merging.

preview

Please click to view the screenshot

small

image

image

medium

image

image

fixed: #3769

@kiaking
Copy link
Member

kiaking commented Apr 18, 2024

Gonna look into this. Needs a bit adjustment.

Copy link

@marcosdissotti marcosdissotti left a comment

Choose a reason for hiding this comment

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

Try use idicomatic css writing to your styles, it's so good ✌️https://github.com/necolas/idiomatic-css

@kiaking
Copy link
Member

kiaking commented Apr 19, 2024

I've updated a few things.

  1. I went with more explicit media query approach on how we grid the layout. The .vp-doc part gets a bit more redundant but team member component style became simpler I guess.
  2. Not related to this PR directly, but found out that <VPTeamPage> component margin top was broken (Vite website having issue, too much margin top) so I fixed it too. Also adjusted how we override these things (:slotted wasn't working as I expected).

@brc-dd
Could you test it 🙏 I've added https://localhost:5173/team-test page to test full team page. Should remove this file before merging.

@kiaking kiaking requested a review from brc-dd April 19, 2024 00:41
@stevending1st
Copy link
Author

stevending1st commented Apr 19, 2024

@media (max-width: 640px)

image


@kiaking When there are more than 3 members on the page and the width is large, it is always displayed in two columns.

image

@kiaking
Copy link
Member

kiaking commented Apr 19, 2024

Oh no 😅 Thanks! I'll fix it.

@kiaking
Copy link
Member

kiaking commented Apr 22, 2024

OK, now I think I've fixed. I went and separated small and medium size styles completely since mixing them up was mixing up my brain 😅

@stevending1st
Copy link
Author

stevending1st commented Apr 24, 2024

OK, now I think I've fixed. I went and separated small and medium size styles completely since mixing them up was mixing up my brain 😅

Thank you! 😉 But there may be some problems:

  1. When size is set to medium, the height is not equal

image

  1. When the layout is home, size is small, but there are only two columns

image

@kiaking
Copy link
Member

kiaking commented Apr 25, 2024

Oh no... thank you for keep testing! I'll continue look into it. Yeah I wasn't testing in home layout too, but not sure why it makes any difference... 🤔

@stevending1st
Copy link
Author

Oh no... thank you for keep testing! I'll continue look into it. Yeah I wasn't testing in home layout too, but not sure why it makes any difference... 🤔

Thanks for the commit. ❤️You don't have to say thank you to me, I filed an issue (#3769) and then fixed it, and it's my responsibility to do that too. Since I don't know any more details, thank you for taking the time to help me refine this PR. ❤️

@brc-dd brc-dd added wip needs author action The PR is not ready yet and removed wip labels Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs author action The PR is not ready yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support centering members in VPTeamPage
4 participants