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

Titel en subtitel gebouwd #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Titel en subtitel gebouwd #7

wants to merge 1 commit into from

Conversation

jezi89
Copy link
Collaborator

@jezi89 jezi89 commented Dec 4, 2024

Ik heb toch geprobeerd een witte en zwarte titel te make met wat hulp.
Ik heb niet de hubspot rich-text widget/module toegevoegd in de span tag, maar dat zou niks uit moeten maken voor de visualisatie en vraag mij af of dit nodig is.

Copy link
Contributor

@novaeeken novaeeken left a comment

Choose a reason for hiding this comment

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

Heeey Jerome, super goed gedaan joh! Het was natuurlijk echt een vuurdoop in React, maar je hebt het super goed opgepakt. Kijk nog even naar de feedback en verbeter de PR!

@@ -0,0 +1,33 @@
.section-title-black {
Copy link
Contributor

Choose a reason for hiding this comment

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

Netjes in kebab-case gedaan, top!

@@ -0,0 +1,33 @@
.section-title-black {
text-align: center;
display: block;
Copy link
Contributor

Choose a reason for hiding this comment

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

Deze is al standaard voor titels en paragrafen, dus die mag weg :)

.section-title-black {
text-align: center;
display: block;
color: var(--color-text);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, goed gebruik gemaakt van de variabelen! 👍

text-align: center;
display: block;
color: var(--color-text);
margin: 0 0 20px 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Oehhhh en een short-hand, lekker bezig!

display: block;
color: var(--color-text);
margin: 0 0 20px 0;
font-weight: bold;
Copy link
Contributor

Choose a reason for hiding this comment

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

De font-weight 600 is eigenlijk standaard voor alle titels in dit project, dus het is beter om die dan bij de algemene font-declaraties te zetten in index.css

}


/*.section-subtitle {*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Deze mag je weghalen!

function TitleSubtitle({title, subTitle, className}) {
return (
<>
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Deze div heeft geen styling gekregen, dus eigenlijk is 'ie hier overbodig!

<div>

<h2 className={className}>

Copy link
Contributor

Choose a reason for hiding this comment

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

Ook hier: no trailing whitespace plz

<span><strong>{title}</strong></span>
</h2>

<p className={className}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Goed opgelost zo met de properties, chapeau! En dat terwijl je nog nooit met React hebt gewerkt 👍 Er is nog een iets geavanceerdere manier om dit te doen, dus mocht je nog een keertje langskomen bij Coding Wednesday, dan kunnen we daar nog eens naar kijken!

/>
</SectionContentWrapper>
<SectionContentWrapper>
<h2>ICT-Cursussen</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

Al deze h2'tjes op de pagina kunnen nu worden vervangen door jouw vette component! 😍

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.

2 participants