Skip to content

Conversation

@carro-barro
Copy link

Carolina Oldertz added 28 commits November 25, 2025 19:02
… with the projects section, i added some info in the json file about the projects
…es the styling with props, started to loop the project.json and display it in the browser, but needs more styling
…nd figuring out the logic, also made a article json and started to add info
…h article.json and made a typography folder with reusable text elements
…ed styling on all sections, need to make minor styling fixes and look at accessibility and stretch goals
Copy link

@Npahlfer Npahlfer left a comment

Choose a reason for hiding this comment

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

Copy link

@qabalany qabalany left a comment

Choose a reason for hiding this comment

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

🎉
Hey! I went through your portfolio and it's looking great!

This is really solid work! The code is clean, organized, and shows you understand React well. The responsive design is thorough, and little touches like proper alt texts show attention to detail.

The suggestions above are all minor polish - nothing is broken, and the portfolio looks professional. Keep up the great work! 🚀

P.S. - Love the cat subscription site with party mode. That creativity really shows your personality! 🐱

Comment on lines +1 to +7
import styled, { createGlobalStyle } from "styled-components"
import { HeroSection } from "./components/hero/HeroSection"
import { TechSection } from "./components/Tech/TechSection"
import { ProjectSection } from "./components/projects/ProjectSection"
import { SkillSection } from "./components/Skills/SkillSection"
import { ArticleSection } from "./components/articles/ArticleSection"
import { FooterSection } from "./components/footer/FooterSection"
Copy link

Choose a reason for hiding this comment

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

Clean imports, nicely organized.

}
`

export const App = () => {
Copy link

Choose a reason for hiding this comment

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

Simple and readable component structure

margin: 0;
}
`

Copy link

Choose a reason for hiding this comment

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

GlobalStyles in the same file keeps things together nicely

@@ -0,0 +1,13 @@
export const theme = {

breakpoints: {
Copy link

Choose a reason for hiding this comment

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

Breakpoints are well organized - mobile first approach is smart

desktop: '1024px',
},

color: {
Copy link

Choose a reason for hiding this comment

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

Centralizing colors makes future changes super easy

@@ -0,0 +1,22 @@
import styled from "styled-components"

const SocialIconLink = styled.a`
Copy link

Choose a reason for hiding this comment

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

Simple, focused component

Comment on lines +15 to +21
return (
<>
<SocialIconLink href={props.href}>
<img src={props.src} alt={props.alt} />
</SocialIconLink>
</>
)
Copy link

Choose a reason for hiding this comment

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

The fragment <> </> isn't needed here since you're returning one element

Comment on lines +6 to +33
<link
rel="icon"
type="image/x-icon"
href="/favicon/favicon.ico"
>
<link
rel="icon"
type="image/png"
sizes="32x32"
href="/favicon/favicon-32x32.png"
>
<link
rel="icon"
type="image/png"
sizes="16x16"
href="/favicon/favicon-16x16.png"
>

<link
rel="manifest"
href="/favicon/site.webmanifest"
>

<link
rel="apple-touch-icon"
sizes="180x180"
href="/favicon/apple-touch-icon.png"
>
Copy link

Choose a reason for hiding this comment

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

Thorough favicon setup for all devices

href="https://fonts.googleapis.com/css2?family=Lato:ital,wght@0,100;0,300;0,400;0,700;0,900;1,100;1,300;1,400;1,700;1,900&family=Montserrat:ital,wght@0,100..900;1,100..900&family=Poppins:ital,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;0,800;0,900;1,100;1,200;1,300;1,400;1,500;1,600;1,700;1,800;1,900&display=swap"
rel="stylesheet"
>
<title>Portfolio</title>
Copy link

Choose a reason for hiding this comment

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

Small SEO suggestion:
The title is generic


Current 
<title>Portfolio</title>

More SEO-friendly
<title>Carolina Oldertz | Frontend Developer</title>

"name": "Weather app",
"image": "https://images.unsplash.com/photo-1520792532857-293bd046307a?ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D&auto=format&fit=crop&w=2370&q=80",
"name": "RECIPE LIBRARY",
"info": "Using spoonaculars API I have fetched a library of recipes that displays ingredients, cooking time and meal type. You can filter and sort by different categories and search for your recipe and aslo with a button click to geta random recipe.",
Copy link

Choose a reason for hiding this comment

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

Tiny typo
"aslo" should be "also"
Also "geta" should be "get a" 😊

@@ -0,0 +1,18 @@
import styled from "styled-components"
Copy link

@Npahlfer Npahlfer Dec 9, 2025

Choose a reason for hiding this comment

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

Instead of defining a set of different Typographies like you have done in here, it would be nicer to define a component that can change (both the styling and the element) depending on the prop. Like we did in class:
https://github.com/Technigo/fall-2025-live-sessions/blob/main/week-14/styled-components/src/components/Typography.jsx

Copy link

Choose a reason for hiding this comment

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

This file can be deleted as you have global styling in the GlobalStyle function and the rest in each component :)

@@ -0,0 +1,13 @@
export const theme = {
Copy link

Choose a reason for hiding this comment

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

You could even expand with more themes here eg. spacing and font styling.

object-fit: cover;
border-radius: 12px;
box-shadow: 0 4px 4px 0 rgba(0, 0, 0, 0.25);
object-position: ${props => (props.$altText === 'Screenshot of RECIPE LIBRARY' ? 'left' : 'center')};
Copy link

@Npahlfer Npahlfer Dec 9, 2025

Choose a reason for hiding this comment

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

Checking for an alt text like this is a bad idea because the alt text might change and then this doesnt work anymore. Better to use another prop on the ProjectCardImage (eg. direction="left")

`

export const ProjectCard = ({ name, info, netlify, github, image, tags, index }) => {
const shouldReverse = index % 2 !== 0
Copy link

Choose a reason for hiding this comment

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

modulo, nice!

},
{
tag: "Toolbox",
info: "Atom\nPostman\nAdobe Photoshop\nAdobe Illustrator\nAdobe InDesign\nAdobe Acrobat\nAdobe Premiere Pro & Rush\nAdobe After Effects\nCamera raw\nBridge\nWordpress\nSitecore (CMS)\nSitevison (CMS)\nShopify\nFigma\nMicrosoft (Word, ppt, etc)\nSlack",
Copy link

Choose a reason for hiding this comment

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

instead of having them separated with a linebreak, use an array! :)

Copy link

Choose a reason for hiding this comment

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

then you dont have to use the split like you do later in SkillInfo

@Npahlfer
Copy link

Npahlfer commented Dec 9, 2025

There are some room for improvements (check my comments) like the expansion of the theme and some logic parts, but overall really good! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants