-
Notifications
You must be signed in to change notification settings - Fork 0
frontend prototype #2
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?
Conversation
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.
Pull request overview
This pull request introduces a frontend prototype for a queue management system using React 19, TypeScript, Vite 7, and Tailwind CSS 4. The application allows users to join a virtual queue, view their position, and see other people waiting in line.
Changes:
- Set up a modern React + TypeScript project with Vite as the build tool
- Implemented queue management functionality with a custom React hook
- Created responsive UI components for queue visualization and user interaction
Reviewed changes
Copilot reviewed 23 out of 26 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Defines project dependencies including React 19.2, Vite 7.2, and Tailwind CSS 4.1 |
| vite.config.ts | Configures Vite with React plugin support |
| tsconfig.*.json | TypeScript configuration files for app and node environments |
| tailwind.config.js | Tailwind CSS configuration with content paths |
| src/hooks/useQueue.js | Custom hook for queue state management (should be TypeScript) |
| src/components/*.tsx | React components for queue UI including header, body, preview, and forms |
| src/main.tsx | Application entry point with React root rendering |
| src/index.css | Global styles with Tailwind imports |
| index.html | HTML entry point for the SPA |
| eslint.config.js | ESLint configuration for TypeScript and React |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ]); | ||
|
|
||
| const [name, setName] = useState(''); |
Copilot
AI
Jan 14, 2026
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.
The state variable 'name' is declared but never used in this hook. It should be removed or exported if needed.
|
|
||
| const addToQueue = (nameFromForm) => { | ||
| const newPerson = { | ||
| id: Date.now(), |
Copilot
AI
Jan 14, 2026
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.
Using Date.now() for generating IDs can lead to collisions if multiple users join in quick succession. Consider using a more robust ID generation method like crypto.randomUUID() or a counter-based approach.
| @@ -0,0 +1,28 @@ | |||
| // src/components/QueuePosition.jsx | |||
|
|
|||
| export function QueuePosition({ currentUser, position, peopleAhead, onLeave }) { | |||
Copilot
AI
Jan 14, 2026
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.
Props are not typed. Consider adding TypeScript prop types for type safety.
| export function QueuePosition({ currentUser, position, peopleAhead, onLeave }) { | |
| export interface QueuePositionProps { | |
| currentUser: { | |
| name: string; | |
| }; | |
| position: number; | |
| peopleAhead: number; | |
| onLeave: () => void; | |
| } | |
| export function QueuePosition({ currentUser, position, peopleAhead, onLeave }: QueuePositionProps) { |
| @@ -0,0 +1,11 @@ | |||
| // src/components/QueueNotification.jsx | |||
|
|
|||
| export function QueueNotification({ peopleAhead }) { | |||
Copilot
AI
Jan 14, 2026
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.
Props are not typed. Consider adding TypeScript prop types for type safety.
| export function QueueNotification({ peopleAhead }) { | |
| interface QueueNotificationProps { | |
| peopleAhead: number; | |
| } | |
| export function QueueNotification({ peopleAhead }: QueueNotificationProps) { |
| export function QueueBody({ | ||
| currentUser, | ||
| position, | ||
| peopleAhead, | ||
| queue, | ||
| onJoin, | ||
| onLeave |
Copilot
AI
Jan 14, 2026
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.
Props are not typed. Consider adding TypeScript prop types for type safety.
| @@ -0,0 +1,54 @@ | |||
| import { useState } from 'react'; | |||
Copilot
AI
Jan 14, 2026
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.
This file should use TypeScript (.tsx extension) to match the project configuration. The project uses TypeScript throughout, as indicated by tsconfig files, but this hook is defined as a JavaScript file.
| }; | ||
| setQueue([...queue, newPerson]); | ||
| setCurrentUser(newPerson); | ||
| setName(''); |
Copilot
AI
Jan 14, 2026
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.
The setName('') call has no effect since 'name' state is not being used anywhere in this hook. This line should be removed.
| @@ -0,0 +1,70 @@ | |||
| export function QueuePreview({ queue, currentUser, onLeave }) { | |||
Copilot
AI
Jan 14, 2026
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.
Props are not typed. Consider adding TypeScript prop types for type safety.
| export function QueuePreview({ queue, currentUser, onLeave }) { | |
| type QueuePerson = { | |
| id: string | number; | |
| name: string; | |
| }; | |
| interface QueuePreviewProps { | |
| queue: QueuePerson[]; | |
| currentUser?: { id: string | number; name: string }; | |
| onLeave: () => void; | |
| } | |
| export function QueuePreview({ queue, currentUser, onLeave }: QueuePreviewProps) { |
|
|
||
| import { useState } from 'react'; | ||
|
|
||
| export function QueueJoinForm({ onJoin }) { |
Copilot
AI
Jan 14, 2026
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.
Props are not typed. Consider adding TypeScript prop types for type safety.
| export function QueueJoinForm({ onJoin }) { | |
| interface QueueJoinFormProps { | |
| onJoin: (name: string) => void; | |
| } | |
| export function QueueJoinForm({ onJoin }: QueueJoinFormProps) { |
andrewsoonqn
left a comment
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.
Looks good for a prototype. Thanks for the good work!
Some tweaks:
- can u rename the folder to just frontend/ ?
- GH copilot left some good comments, can u make changes accordingly
| return ( | ||
| <div className="bg-[#1e2936] p-6"> | ||
| <h1 className="text-2xl font-bold text-white text-center"> | ||
| NUSC Queuebot SOSD |
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.
No need SOSD
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.
Add setup instructions
yihao03
left a comment
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.
At its current stage, the UI seems to be optimised for mobile view which is fine, assuming that most users would likely access this app on their phone. However, please also implement responsive design for bigger screens to fully utilize the screen real estate.
There could also be privacy concerns of showing everyone the full name of people in the queue. I am not sure how relevant that is.
| export function QueueHeader() { | ||
| return ( | ||
| <div className="bg-[#1e2936] p-6"> | ||
| <h1 className="text-2xl font-bold text-white text-center"> | ||
| NUSC Queuebot SOSD | ||
| </h1> | ||
| </div> | ||
| ); | ||
| } |
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.
avoid premature optimisation/abstraction. same applies to the footer and other components that are only used once
| const [queue, setQueue] = useState([ | ||
| { id: 1, name: "Alice" }, | ||
| { id: 2, name: "Bob" }, | ||
| { id: 3, name: "Charlie" }, | ||
| { id: 4, name: "Dave" }, | ||
| { id: 5, name: "Ellie" }, | ||
| { id: 6, name: "Florence" }, | ||
| { id: 7, name: "Gavin" }, | ||
| { id: 8, name: "Hendrick" }, | ||
|
|
||
|
|
||
| ]); |
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.
understand that this is a mockup data for the queue. however try to be consistent and use typescript over all the files. A proper type should be defined for members in the queue too. Consider including other useful fields like time joined/group size etc
| export default function App() { | ||
| const { | ||
| queue, | ||
| currentUser, | ||
| position, | ||
| peopleAhead, | ||
| addToQueue, | ||
| removeFromQueue | ||
| } = useQueue(); | ||
|
|
||
| return ( | ||
| <div className="min-h-screen bg-[#0e1621] flex items-center justify-center p-4"> | ||
| <div className="w-full max-w-[375px] h-[812px] bg-[#17212b] rounded-3xl shadow-2xl overflow-hidden flex flex-col"> | ||
|
|
||
| {/* Fixed Header */} | ||
| <QueueHeader /> | ||
|
|
||
| {/* Conditional Notification */} | ||
| {currentUser && position > 0 && position <= 3 && ( | ||
| <QueueNotification peopleAhead={peopleAhead} /> | ||
| )} | ||
|
|
||
| {/* Scrollable Body */} | ||
| <QueueBody | ||
| currentUser={currentUser} | ||
| position={position} | ||
| peopleAhead={peopleAhead} | ||
| queue={queue} | ||
| onJoin={addToQueue} | ||
| onLeave={() => removeFromQueue(currentUser?.id)} | ||
| /> | ||
|
|
||
| {/* Fixed Footer */} | ||
| <QueueFooter /> | ||
|
|
||
| </div> | ||
| </div> | ||
| ); | ||
| } |
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.
avoid writing pages in app.tsx unless you are certain that this app will only have one page. However this app would likely have different pages e.g. user view and admin view. Please set up proper project structure and routing logic.
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.
Please update the README properly. include project brief and introduction. only include necessary details in dev/contributing section e.g. setup dev environment and notable tech decisions
| <div className="bg-[#1e2936] p-6"> | ||
| <h1 className="text-2xl font-bold text-white text-center"> | ||
| NUSC Queuebot SOSD | ||
| </h1> | ||
| </div> |
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.
would this eventually become a dynamic component? i.e. display the event title as configured by the event host
No description provided.