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

Created lobby view + implemented standard components #26

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

Conversation

mit3r
Copy link
Collaborator

@mit3r mit3r commented Dec 29, 2024

No description provided.

@mit3r mit3r linked an issue Dec 29, 2024 that may be closed by this pull request
@mit3r mit3r requested a review from Aleks334 December 29, 2024 23:22
@Aleks334
Copy link
Member

I've made small contribution. Fixed issue with missing disabled attribute on button element, moved clsx dep to client dir, etc. I saw that you currently don't support touch devices in Seat component (i can change selection but it's hard also due to swipe feature). Moreover in my opinion i would extend the margin of error when choosing seat (when i use mouse i have to be precise in order to select seat).

Copy link
Member

@Aleks334 Aleks334 left a comment

Choose a reason for hiding this comment

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

I added my comment in conversation tab.

@mit3r mit3r self-assigned this Dec 30, 2024
@mit3r mit3r requested a review from Aleks334 January 9, 2025 22:02
@mit3r
Copy link
Collaborator Author

mit3r commented Jan 9, 2025

I fixed touch and mouse events and i split seats tab into more elements to maintain readability


return (
<div className="relative flex h-full w-full items-center justify-center">
{Children.map(props.children, (child, i) => {
Copy link
Member

@Aleks334 Aleks334 Jan 13, 2025

Choose a reason for hiding this comment

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

I suggest using another way that doesn't require the use of Children api. It's quite rare and even in the docs they don't recommend it: https://react.dev/reference/react/Children

Copy link
Member

@Aleks334 Aleks334 left a comment

Choose a reason for hiding this comment

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

It works awesome now. I saw that you suppressed linter, but I also think that's ok in this case.

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.

Create lobby view
2 participants