Button-complete #6

RallaPirate · 2024-12-04T16:29:57Z

No description provided.

RallaPirate · 2024-12-04T16:30:47Z

Check onze buttons 💯

novaeeken

Heeeey Rob en Gwendolyn, super goed werk geleverd! 🥳 Kijk nog eens goed naar de opmerkingen, als je daarmee aan de slag gaat maak je er een super mooi herbuikbaar component van (dat hoeft natuurlijk niet gelijk nu, je mag ook wachten tot de volgende coding wednesday!)

novaeeken · 2024-12-04T16:35:25Z

src/components/button/Button.jsx

+        >
+            {label}
+            {arrow === true && <ArrowRight size={25}/>}
+


Zeg... Wat doet al deze witruimte hier?

novaeeken · 2024-12-04T16:35:52Z

src/components/button/Button.jsx

+
+function Button({label, name, onClick, arrow}) {
+    return (
+        <button type="button"


En wat nou als ik een button wil met type=submit?

Ik heb de typing weggehaald, zodat de gebruiker zelf kan kiezen welke functie de button heeft.

novaeeken · 2024-12-04T16:36:03Z

src/components/button/Button.jsx

+    return (
+        <button type="button"
+                className={name}
+                onClick={onClick}


Uitstekend!

novaeeken · 2024-12-04T16:37:02Z

src/components/button/Button.jsx

+function Button({label, name, onClick, arrow}) {
+    return (
+        <button type="button"
+                className={name}


Dit werkt al top, maar wat nog beter zou zijn is om twee of drie varianten te maken. Dus iemand geeft mee "primary" of "secundary" en wij koppelen in de button dan de juiste CSS-class daaraan. Anders moeten we de CSS telkens opnieuw schrijven voor iedere gebruikte button...

Ik heb in de Button.css de benaming aangpast naar .button-primary, .button-secundary en .button-tertiary.

novaeeken · 2024-12-04T16:38:42Z

src/components/navbar/Navbar.jsx

@@ -16,7 +17,7 @@ function Navbar() {
 			</ul>
 			<div className="navbar-controls">
 				<p>Student login</p>
-				<button type="button">Inschrijven</button>
+				<Button name="button2" label="Inschrijven" onClick={()=> console.log("Je hebt geklikt!")} arrow={false} />


In plaats van name zou variant een goed idee zijn! En dan kan iemand kiezen tussen primary, secondary of inverted bijvoorbeeld

I has it 'le fixed'

src/components/button/Button.css

novaeeken · 2024-12-05T09:25:20Z

src/components/button/Button.css

+    width: max-content;
+    min-width: 150px;
+    display: flex;
+    justify-content: flex-start;


Dit is de default waarde van justify-content, dus kun je weglaten :)

novaeeken · 2024-12-05T09:25:56Z

src/components/button/Button.css

+    border-radius: 7px;
+    padding: 12px 27px;
+    box-sizing: border-box;
+    width: max-content;


Weten jullie ook wat dit doet? Of heb je die gewoon overgenomen van de echte code? ;)

Code aangepast, tevens de arrow een left-margin van 0.5 em meegegeven voor de ruimte.

novaeeken · 2024-12-05T09:26:52Z

src/components/button/Button.css

+    cursor: pointer;
+}
+
+.button3 {


Deze class bevat nu heel veel dubbele CSS in vergelijking met de andere button-classes. Kijk eens of je de CSS efficienter kunt opschrijven met een basisclass die geldt voor alle buttons, en dan extra classes met de properties die anders zijn, zoals font-size, kleur of borders!

Hmm ik zie niet heel veel wat exact gelijk blijft, maar ik snap zeker wat je bedoeld.

novaeeken · 2024-12-05T09:31:46Z

src/components/button/Button.jsx

+import './Button.css'
+import {ArrowRight} from "@phosphor-icons/react";
+
+function Button({label, name, onClick, arrow}) {


Op basis van true of false die binnenkomt onder arrow ga je bepalen of je het icoon wel of niet wil weergeven. Bij boolean properties leest het altijd het fijnst als je de property has... of is... noemt. Denk bijvoorbeeld aan hasIcon, dat maakt het een stuk duidelijker voor collega's!

novaeeken · 2024-12-05T09:41:34Z

src/components/button/Button.css

+.button1 {
+    font-size: 16px;
+    background: #171717;
+    border: 3px solid #171717;


Oh en gebruik je hier nog even de CSS-kleur-variabelen voor uit index.css?

novaeeken

Heeey jongens, we zijn er bijna! Kijk nog even naar de laatste kleine dingetjes en dan kan 'ie door :)

novaeeken · 2024-12-19T09:55:38Z

src/App.jsx

+                            nostrum omnis quae, quam quis rem ut.</p>
+                        <Button variant="button-tertiary"
+                                onClick={() => console.log("Je hebt geklikt!")
+                                }


Deze staat een beetje op een gekke plek, kan 'ie terug op de vorige regel?

novaeeken · 2024-12-19T09:57:01Z

src/App.jsx

+                                label="Bekijk onze opleidingen"
+                                arrow={true}
+                        >
+                        </Button>


Het button component neemt geen children, dus in dat geval zou 'ie self-closing moeten zijn. Echter, voelt het fijner om een button te gebruiken zoals je 'm in de HTML gewend bent. Je zou ervoor kunnen kiezen om de label-property te vervangen door de children property, zodat je de button zo kunt gebruiken:
<Button arrow={true} variant="button-tertiary" onClick={}>Bekijk onze opleidingen</Button

novaeeken · 2024-12-19T09:58:41Z

src/components/button/Button.jsx

+    return (
+        <button className={variant}
+                onClick={onClick}>
+            {label}


als je label hier (en op regel 4) vervangt door children kun je 'm gebruiken zoals ik heb beschreven in de vorige comment!

novaeeken · 2024-12-19T10:01:34Z

src/components/button/Button.jsx

+        <button className={variant}
+                onClick={onClick}>
+            {label}
+            {hasArrow === true && <ArrowRight size={25}/>}


Hier checken jullie expliciet (wat prima is) en het zou ook impliciet kunnen, gezien de hasArrow waarde altijd een boolean is. Dus hasArrow && <ArrowRight size={25}/> werkt ook :)

novaeeken · 2024-12-19T10:03:59Z

src/components/button/Button.jsx

+
+function Button({label, variant, onClick, hasArrow}) {
+    return (
+        <button className={variant}


Mocht je het nou extra, éxtra netjes willen doen, kun je switch statement gebruiken om de className te bepalen. Want als ik een spelfout maak in de variant die ik meegeef, dan krijgt de button geen styling omdat de CSS-class nergens mee matcht. Door de className op te bouwen met een switch statement (tussen regel 4 en 5) kun je ook altijd terugvallen op iets, als geen van de opties matcht.

novaeeken · 2024-12-19T10:05:20Z

src/components/button/Button.css

+    cursor: pointer;
+}
+
+button > svg {


👍 👍 👍 Alleen kun je van button beter een specifieke class-selector maken, zoals:
.button-primary > svg, .button-secondary > svg, .button-tertiary > svg {}

novaeeken · 2024-12-19T10:07:37Z

src/components/button/Button.css

+    display: flex;
+    align-items: center;
+    cursor: pointer;
+}


Waarschijnlijk valt het jullie ook op dat veel van de properties nu herhaald worden (dubbele CSS). Dus als we nu besluiten dat buttons minder padding moeten krijgen, moeten we dat op drie regels aanpassen (niet ideaal). Door selectors te groepperen, kun je de declaraties die alle buttons moeten hebben 1 keer schrijven:

.button-primary, .button-secundary, .button-tertiary { zet hier alles dat hetzelfde is }

.button-primary { zet hier alle aanvullingen die alleen voor primary gelden }
.button-secondary { zet hier alle aanvullingen die alleen voor secondary gelden }
.button-tertiary { zet hier alle aanvullingen die alleen voor tertiary gelden }

Button-complete

3fb4201

novaeeken self-requested a review December 4, 2024 16:34

novaeeken requested changes Dec 5, 2024

View reviewed changes

novaeeken reviewed Dec 5, 2024

View reviewed changes

Button is happy de peppy en klaar voor gebruik

d78f3cb

novaeeken requested changes Dec 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Button-complete #6

Button-complete #6

RallaPirate commented Dec 4, 2024

RallaPirate commented Dec 4, 2024

novaeeken left a comment

novaeeken Dec 4, 2024

E-clpz Dec 18, 2024

novaeeken Dec 4, 2024

E-clpz Dec 18, 2024

novaeeken Dec 4, 2024

novaeeken Dec 4, 2024

E-clpz Dec 18, 2024

novaeeken Dec 4, 2024

E-clpz Dec 18, 2024

novaeeken Dec 5, 2024

E-clpz Dec 18, 2024

novaeeken Dec 5, 2024

E-clpz Dec 18, 2024

novaeeken Dec 5, 2024

E-clpz Dec 18, 2024

novaeeken Dec 5, 2024

E-clpz Dec 18, 2024

novaeeken Dec 5, 2024

novaeeken left a comment

novaeeken Dec 19, 2024

novaeeken Dec 19, 2024

novaeeken Dec 19, 2024

novaeeken Dec 19, 2024

novaeeken Dec 19, 2024

novaeeken Dec 19, 2024

novaeeken Dec 19, 2024

Button-complete #6

Are you sure you want to change the base?

Button-complete #6

Conversation

RallaPirate commented Dec 4, 2024

RallaPirate commented Dec 4, 2024

novaeeken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

novaeeken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment