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

[WIP]: Refactor team page #94

Closed
wants to merge 18 commits into from
Closed

[WIP]: Refactor team page #94

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
0004bc3
fix: delete cypress dep, cause error in package downloading
rennavittorio Sep 4, 2023
1996780
enh: add new team member and refactor img folder name and team-member…
rennavittorio Sep 4, 2023
725994a
fix: delete router-link from whole card
rennavittorio Sep 4, 2023
6151da8
enh(TeamPage): add MemberCard and SocialIcons component
rennavittorio Sep 4, 2023
0ae6958
chore: refactor script positioning and delete old comments
rennavittorio Sep 5, 2023
c88b29f
refactor(team-member): refactor using component and p
rennavittorio Sep 5, 2023
d948b0f
feat: create reusable heading component for page title
rennavittorio Sep 8, 2023
f7a5150
refactor: refactor team page with unocss and heading
rennavittorio Sep 8, 2023
3be1717
fix: fix h1, align to convention
rennavittorio Sep 10, 2023
4605522
refactor: refactor components style with unocss
rennavittorio Sep 10, 2023
17f2167
refactor: refactor team-member page with unocss
rennavittorio Sep 11, 2023
b173c39
fix(package): add cypress dependency (align to project)
rennavittorio Sep 20, 2023
666e2cc
fix(img): rename folder into team-members
rennavittorio Sep 20, 2023
bb20108
fix(img): rename members team img (align to project)
rennavittorio Sep 20, 2023
c165a85
chore(config): add brand color to unocss config
rennavittorio Sep 20, 2023
bd94805
fix(components): align unocss classes to project, manage light-dark m…
rennavittorio Sep 20, 2023
13a889d
fix(components): align array type to project standard
rennavittorio Sep 20, 2023
ed76e4b
fix(views): align unocss classes and position to project standard
rennavittorio Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,461 changes: 88 additions & 1,373 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
"dependencies": {
"@unocss/reset": "^0.54.2",
"@vueuse/core": "^10.1.0",
"cypress": "^12.17.3",
Readpato marked this conversation as resolved.
Show resolved Hide resolved
"register-service-worker": "^1.7.2",
"vue": "^3.2.45",
"vue-i18n": "^9.2.2",
Expand Down
0 public/img/_dev/Angy.jpg → public/img/team_members/angelabusato.jpg
View file
Open in desktop
Readpato marked this conversation as resolved.
Show resolved Hide resolved
File renamed without changes
0 public/img/_dev/Danersound.jpg → public/img/team_members/danersound.jpg
View file
Open in desktop
File renamed without changes
0 public/img/_dev/davideimola.jpg → public/img/team_members/davideimola.jpg
View file
Open in desktop
File renamed without changes
0 public/img/_dev/gabrielepuliti.png → public/img/team_members/gabrielepuliti.png
View file
Open in desktop
File renamed without changes
0 public/img/_dev/lorenzopieri.png → public/img/team_members/lorenzopieri.png
View file
Open in desktop
File renamed without changes
0 public/img/_dev/mikilombardi.jpeg → public/img/team_members/mikilombardi.jpeg
View file
Open in desktop
File renamed without changes
0 public/img/_dev/nicpuppa.png → public/img/team_members/nicpuppa.png
View file
Open in desktop
File renamed without changes
Binary file added public/img/team_members/profile-ph.png
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Where does this image come from and what is its purpose?

Copy link
Member

Choose a reason for hiding this comment

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

On further investigation, this image belonged to team member Lorenzo Bugli, and now it's broken:

image

I would check out what has happened over there =]

Copy link
Contributor Author

@rennavittorio rennavittorio Sep 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This was a possible idea to manage members without photos (ie. a placeholder), and accelerate the insertion of members profile into the website, just a propose, if we want to handle these cases differently, I'll align the page :)

Copy link
Member

Choose a reason for hiding this comment

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

I like this idea! You can set it as default when on the component if no src is passed to the img.
I'm still seeing the Lorenzo Bugli image broken though!

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
50 changes: 50 additions & 0 deletions src/components/MemberCard.vue
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

There's some confusion on how the UnoCSS rules are declared and utilized in this PR: for example, we have rules like: px-15px, py-5px, w-128px,mx-2em, text-1rem utilized in this component which mix all the possible types of units that we can utilize in CSS. This plays against the flexibility of UnoCSS and their already baked in values in rem units.

  • w-128px shouldn't be utilized like this, because you could've already declared it like w-32 which if you multiply 32 * 4 = 128 it will give you your desired result! You can also check this specific documentation of TailwindCSS width which allows you to see how all this type of frameworks have an approach of 4 point grid in their design.

  • py-5px should be avoided because it doesn't fit into the 4pt grid, you could easily use py-1 to obtain 4px.

  • px-15px should be avoided as well, you could utilize px-4 which would equal to 16px and and by doing that avoiding to use px on an UnoCSS rule.

  • mx-2em the use of em is discouraged as it doesn't fit well with the 4pt grid. You should stick to rem values.

  • text-1rem could also be rewritten as text-base. See explicit font size documentation of Tailwind here. Even with the documentation read, you shouldn't be applying this rule because the global font value is already 16px (which equals to 1rem) check it out on your own App.vue file!

You can check out this recent component that has been updated to utilize UnoCSS so you can see how there's no need to implement all different types of unit values on the utility rules if we already stand by a system =]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix(components): align unocss classes to project, manage light-dark m…
fixed
(also big thanks for all the docs linked! <3 )

Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<script setup lang="ts">
import { useI18n } from 'vue-i18n'
import SocialIcons from './SocialIcons.vue'

defineProps<{
member: string
}>()

const socials = ['github', 'linkedin', 'twitter', 'website']

const { t } = useI18n()
</script>

<template>
<div
class="overflow-hidden rounded-5px bg-#d8dee9 dark:bg-#3b4252 p-32px font-sans"
data-test="team-card"
:data-test-member-name="`team-member-${member}`"
>
<div class="relative z-0 flex h-auto items-center justify-center rounded-full mx-2em my-auto text-center">
<div
:data-test="`team-member-${member}-index-photo`"
class="block w-128px h-128px rounded-full bg-center bg-cover"
:style="`background-image: url( ${$t(`team.${member}.image`)} );`"
/>
</div>
<div class="relative z-1 flex flex-col items-center justify-center gap-4px">
<h2
class="text-1.5rem font-700"
:data-test="`team-member-${member}-name`"
>
{{ $t(`team.${member}.name`) }}
</h2>

<SocialIcons :member="member" :socials="socials" />

<router-link
class="w-fit px-15px py-5px border-1px border-solid border-transparent rounded-full bg-#586379 text-#fff text-1rem font-700 hover:bg-#2e3440"
Copy link
Member

@Readpato Readpato Sep 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

No need to declare text-#fff this is simply text-white see Text Color on Tailwind docs.

This rule is complicated: hover:bg-#2e3440 what happens if one day we want to change this element into a component but we want to have different colours on hover? We would have to go to each component and modify it. Better to declare a global SCSS class that allows you to declare in the <style scoped lang="scss"> and their respective classes to apply it.

Also, this colours have already been declared on the nord.scss file which is then imported globally so you could easily add them on a specific SCSS class:

<style scoped lang="scss">
.my-class {
  &:hover {
    background-color: $dark-bg-primary;
  }
}
</style>

Also, does this component implement the change between light/dark theme properly?

Remember, refactoring to UnoCSS doesn't mean SCSS shouldn't be used at all!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

chore(config): add brand color to unocss config
What do you think about this solution, and moving project variables into unocss config?
Let's take full advantage of this cool tool! @Readpato

:data-test="`team-member-${member}-page-link`"
:to="{ name: 'TeamMember', params: { member } }"
>
{{ t('redirect.profile') }}
</router-link>
</div>
</div>
</template>

<style lang="scss" scoped>
Readpato marked this conversation as resolved.
Show resolved Hide resolved

</style>
25 changes: 25 additions & 0 deletions src/components/SocialIcons.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<script setup lang="ts">
defineProps<{
member: string
socials: Array<string>
Readpato marked this conversation as resolved.
Show resolved Hide resolved
}>()
</script>

<template>
<div class="w-fit">
<div v-for="social in socials" :key="social" class="inline-block">
<a
v-if="$t(`team.${member}.${social}_url`).length > 1"
class="inline-block m-0.2em group"
:data-test="`team-member-${member}-${social}`"
:href="$t(`team.${member}.${social}${social !== 'website' ? '_url' : ''}`)" target="_blank"
>
<i :class="[`text-1.2em ${social !== 'website' ? `fab fa-${social}` : 'fa fa-cloud'} group-hover:text-#2e3440`]" />
</a>
</div>
</div>
</template>

<style scoped lang="scss">

</style>
102 changes: 63 additions & 39 deletions src/i18n/messages.ts
View file
Open in desktop

Large diffs are not rendered by default.

161 changes: 26 additions & 135 deletions src/views/TeamMember.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { RouteParamValue } from 'vue-router'
import { useRoute } from 'vue-router'

import { useI18n } from 'vue-i18n'
import SocialIcons from '@/components/SocialIcons.vue'

const { t } = useI18n()

Expand All @@ -18,156 +19,46 @@ const getTeamMemberName = () => {
member.value = route.params.member as RouteParamValue
}

const socials = ['github', 'linkedin', 'twitter', 'website']

onMounted(() => {
getTeamMemberName()
})
</script>

<template>
<!-- TODO: Add loader -->
<span v-if="!member">Loading...</span>
<div v-else class="team">
<div class="container">
<div class="content">
<h1 data-test="member-page-name" class="head-1">
{{ $t(`team.${member}.name`) }}
</h1>
<div>
<div
class="photo"
:style="`background-image: url(${$t(`team.${member}.image`)});`"
data-test="member-page-photo"
/>
</div>
<div class="socialIcons !p-2">
<a
v-if="$t(`team.${member}.github_url`).length > 1"
:href="$t(`team.${member}.github_url`)" target="_blank"
data-test="member-page-github"
>
<i class="mobile-menu-icon fab fa-github" />
</a>
<a
v-if="$t(`team.${member}.linkedin_url`).length > 1"
:href="$t(`team.${member}.linkedin_url`)" target="_blank"
data-test="member-page-linkedin"
>
<i class="mobile-menu-icon fab fa-linkedin" />
</a>
<a
v-if="$t(`team.${member}.twitter_url`).length > 1"
:href="$t(`team.${member}.twitter_url`)" target="_blank"
data-test="member-page-twitter"
>
<i class="mobile-menu-icon fab fa-twitter" />
</a>
<a
v-if="$t(`team.${member}.website`).length > 1"
:href="$t(`team.${member}.website`)" target="_blank"
data-test="member-page-website"
>
<i class="mobile-menu-icon fa fa-cloud" />
</a>
</div>

<!-- TODO: Make this a proper <p> -->
<div class="container p-32px max-w-700px m-auto text-center flex flex-col justify-center items-center gap-16px">
<h1 data-test="member-page-name" class="text-2rem font-700">
{{ $t(`team.${member}.name`) }}
</h1>
<div>
<div
class="description !p-2"
data-test="member-page-description"
v-html="$t(`team.${member}.description`)"
:data-test="`team-member-${member}-index-photo`"
class="block w-128px h-128px rounded-full bg-center bg-cover"
:style="`background-image: url( ${$t(`team.${member}.image`)} );`"
/>

<router-link
class="user-profile-link"
:to="{ name: 'Team' }" data-test="nav-team-page-link"
>
{{ t('redirect.back') }}
</router-link>
</div>

<SocialIcons :member="member" :socials="socials" />

<p
class=""
data-test="member-page-description"
v-html="$t(`team.${member}.description`)"
/>

<router-link
class="w-fit px-15px py-5px border-1px border-solid border-transparent rounded-full bg-#586379 text-#fff text-1rem font-700 hover:bg-#2e3440"
Copy link
Member

Choose a reason for hiding this comment

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

Unify classes without utilizing px and hex colours in line. See feedback left on other comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix(components): align unocss classes to project, manage light-dark m…
fixed

:to="{ name: 'Team' }" data-test="nav-team-page-link"
>
{{ t('redirect.back') }}
</router-link>
</div>
</div>
</template>

<style scoped lang="scss">
.photo {
display: inline-block;
width: 128px;
height: 128px;
border-radius: 100%;
background-position: center;
background-size: cover;
text-align: center;
}

.team {
.content {
max-width: 700px;
padding: 1.5em 3em;
margin: auto;
text-align: center;
}
}

.socialIcons {
padding: 1em;

a {
display: inline-block;
margin: 0.2em;

.mobile-menu-icon {
font-size: 1.2em;
}

&:hover .mobile-menu-icon {
color: #2e3440;
}
}
}

.description {
padding: 1em 0;
margin-bottom: 20px;
}

.user-profile-link {
padding: 5px 15px;
border: 1px solid transparent;
border-radius: 999px;
margin: 0 auto;
background-color: #586379;
color: #fff;
font-weight: bold;

&:hover {
background-color: #2e3440;
}
}

@media (width >= 56.25em) {
.event {
.cta {
display: block;

.btn {
margin: 0 5px;

&:nth-of-type(1) {
margin-left: 0;
}
}
}
}
}

.#{$dark-mode-class} {
.socialIcons {
a {
&:hover .mobile-menu-icon {
color: #586379;
}

}
}
}
</style>
Loading