-
Notifications
You must be signed in to change notification settings - Fork 2
komponent kafel trybu gry #21
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
f5a1bf0
f7d70db
fc5cbd1
03612a0
f7f6eaa
02126f0
54b59c4
3fcba96
d98166e
dd9d06f
6cb56de
48e63b3
15a3840
11c7d0a
af02555
02e925e
beddf83
7796534
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| type ContainerComponentLabel = HTMLElement | string | ||
|
|
||
| export const gameMode = ( | ||
| label: ContainerComponentLabel, | ||
| onClick: () => void, | ||
| parentId?: string, | ||
| className?: string | ||
| ): HTMLElement => { | ||
| const component: HTMLElement = document.createElement('button')! | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Myślę że div byłby lepszym wyborem, z tego względu że po kliknięciu nie będzie "odciśnięty" oraz jest blokowy. |
||
|
|
||
| label = HTMLElement | ||
| ? (component.innerHTML = `${label}`) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metoda appendChild będzie tutaj chyba bardziej wydajna. |
||
| : /(jpg|gif|png|JPG|GIF|PNG|JPEG|jpeg)$/.test(`${label}`) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sprawdzenie czy string jest obrazkiem zrobiłam już w moim komponencie. Przeniosę tę funkcję gdzieś indziej żebyśmy obie mogły z niej korzystać. |
||
| ? (component.style.backgroundImage = `url(${label})`) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wydaje mi się że podwójna ternary operation nie jest zbyt dobrze czytelna. |
||
| : (component.innerText = `${label}`) | ||
|
|
||
| component.addEventListener('click', onClick) | ||
|
|
||
| const parentEl = document.getElementById(`${parentId}`) | ||
| component.classList.add(`${className}`) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parametry parentId oraz className masz zaznaczone jako niewymagane parametry, więc najlepiej byłoby najpierw sprawdzić czy taka zmienna istnieje zanim ją wywołasz.. w każdym razie na pewno dla className, bo nie jest przypisana do osobnej zmiennej. |
||
|
|
||
| return parentEl ? parentEl.appendChild(component) : component | ||
| } | ||
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.
Proponuję odwrócić kolejność: className a następnie parentId. Najprawdopodobniej częściej będzie używana klasa bez podania odnośnika rodzica więc będzie można podać wówczas mniej parametrów przy wywołaniu komponentu.