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

Feature/informationcard #5

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

Conversation

MorikuMarieke
Copy link
Collaborator

Hee Nova,

Ik heb het component werkend, het is nog niet helemaal klaar.

  • Tekst in p en h3 elementen moet nog worden toegevoegd
  • Link in button moet nog worden toegevoegd via onClick, maar ik weet nog niet welke pagina's gekoppeld moeten worden.

Groetjes,

Marieke

hidden banana 🍌

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.

Super goed gedaan, nog een paar kleine dingetjes en dan mag 'ie door!!

src/App.css Outdated
@@ -12,6 +12,13 @@
margin: 0 auto;
}

.info-card-wrapper {
display: flex;
/*justify-content: space-evenly;*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Als het uitgecommend is, kan het ook weg 🤓

src/App.jsx Outdated Show resolved Hide resolved
src/components/informationCard/InformationCard.css Outdated Show resolved Hide resolved
src/components/informationCard/InformationCard.jsx Outdated Show resolved Hide resolved
src/components/informationCard/InformationCard.css Outdated Show resolved Hide resolved
src/components/informationCard/InformationCard.css Outdated Show resolved Hide resolved
src/components/informationCard/InformationCard.css Outdated Show resolved Hide resolved
src/components/informationCard/InformationCard.css Outdated Show resolved Hide resolved
@@ -0,0 +1,32 @@
.infoCard {
flex: 0 1 calc(50% - 30px);
Copy link
Contributor

Choose a reason for hiding this comment

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

We missen nog een media-query voor mobiel die ervoor zorgt dat de kaarten 100% breed worden :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Als volgt opgelost:

@media screen and (max-width: 768px) {
    /*this screen width includes larger phones*/
    .info-card {
        flex: 0 1 calc(100% - 1.4rem);
    }

}

@MorikuMarieke
Copy link
Collaborator Author

Ik heb de veranderingen verwerkt, n.a.v. onderzoek naar de originele Novi website heb ik nog wat andere wijzigingen gedaan:

  • h3 titel vervangen voor h4.
  • ipv gap heb ik toch margin gebruikt op de .info-card, omdat ik een probleem signaleerde met een gap balk aan de rechterzijde, ik zag dat ze dit op de originele website met margin hadden gedaan ipv gap. Zie bijgevoegde afbeelding.
  • Ik ben begonnen met het gebruik van em en rem ipv px.
    Schermafbeelding 2024-12-15 141116

Dank voor alle feedback, ik heb er veel van geleerd!

@MorikuMarieke
Copy link
Collaborator Author

Hallo Nova,

Ik heb het component klaar, officiele tekst vanuit Novi website geïmporteerd. Ik ben benieuwd of het component zo goed is en op de juiste manier in de website is ingebouwd. Ik heb nog niks met de links gedaan naar andere pagina's, misschien iets leuks om te doen voor aankomende coding wednesday!

Groetjes,

Marieke

src/App.jsx Outdated
@@ -2,6 +2,7 @@ import './App.css'
import SectionContentWrapper from './components/sectionContentWrapper/SectionContentWrapper.jsx';
import novilogo from './assets/novi-logo-color-transparent.png';
import Navbar from './components/navbar/Navbar.jsx';
import InformationCard from "./components/informationCard/InformationCard.jsx";
Copy link
Contributor

Choose a reason for hiding this comment

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

Plz use single quotes

<h3 className="info-card-title">{infoCardTitle}</h3>
<p className="info-card-content">{infoCardContent}</p>
{experienceNeeded && <p className="info-card-experience-tag"><em>Enige IT-ervaring vereist</em></p>}
<button
Copy link
Contributor

Choose a reason for hiding this comment

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

@MorikuMarieke deze nog

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