-
Notifications
You must be signed in to change notification settings - Fork 0
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
Modal component #111
base: frontend
Are you sure you want to change the base?
Modal component #111
Conversation
A-Wodnicki
commented
Aug 3, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zapisuję tyle ile zdążyłem przejrzeć na szybko 😅 Komponent jeszcze do przedyskutowania 😉 Ale ogólnie gratulacje za solidnie przygotowany kod i ciekawe rozwiązania!
* - [dialog](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog) | ||
* */ | ||
function Modal({ title, children, modal, variant, open, onClose }) { | ||
let showConfirm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Możesz tutaj użyć ternary operator
i usunąć tego let
a. Chyba, że masz jakiś głębszy zamysł z użyciem let
w tym przypadku 😅
let showConfirm | |
const showConfirm = variant === "confirm" ? true : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zrobiłem tak w razie wypadku gdyby w przyszłości miałoby się pojawić więcej wariantów, niżej jest switch
, który by tym potem sterował
ui/src/components/Modal/Modal.jsx
Outdated
}, [open]) | ||
|
||
return ( | ||
<dialog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nie jestem przekonany czy użycie elementu dialog
jest najlepsze, ze względu na to, że nie wszędzie jest to jeszcze wspierane. Przedyskutujmy to na najbliższym spotkaniu 😅
console.log('clicked') | ||
}} | ||
> | ||
try clicking me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ten button jest chyba niepotrzebny 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chciałem jakoś pokazać, że można wchodzić w interakcję z resztą strony, kiedy prop modal
ma wartość false