Conversation
performance (youtube removal, etc...)
There was a problem hiding this comment.
Pull Request Overview
This PR implements various UI improvements and bug fixes across multiple components, focusing on enhancing user experience through better formatting, debouncing, and mobile responsiveness.
Key changes include:
- Refactored Home page CTA section with new background styling and removed video embed
- Added debouncing to DepartureTable input fields (callsign, stand, squawk) to reduce unnecessary updates
- Enhanced AcarsChartDrawer with search functionality and mobile-optimized split view
- Improved code formatting and readability across multiple components
- Fixed activity time tracking logic in session users websocket to prevent double-counting
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/Home.tsx | Redesigned CTA section with new background image handling and simplified layout |
| src/pages/ACARS.tsx | Improved header responsive layout and replaced custom buttons with Button component |
| src/components/tools/Toolbar.tsx | Removed connection status text display |
| src/components/tools/ContactAcarsSidebar.tsx | Added Enter key handler for sending messages |
| src/components/tools/ChatSidebar.tsx | Removed online user count display and adjusted message bubble max width |
| src/components/tables/DepartureTable.tsx | Implemented debouncing for callsign, stand, and squawk inputs to improve performance |
| src/components/buttons/UserButton.tsx | Code formatting improvements and removed unused Link import |
| src/components/acars/AcarsChartDrawer.tsx | Added search functionality and mobile-responsive chart viewing |
| src/components/Navbar.tsx | Removed duplicate navigation links from mobile menu |
| server/websockets/sessionUsersWebsocket.ts | Fixed activity time calculation to prevent double-counting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| <div | ||
| className="absolute inset-0" | ||
| style={{ | ||
| backgroundImage, | ||
| backgroundSize: 'cover', | ||
| backgroundPosition: 'center', | ||
| backgroundRepeat: 'no-repeat', | ||
| opacity: customLoaded ? 1 : 0, | ||
| transition: 'opacity 0.5s ease-in-out', | ||
| }} | ||
| /> |
There was a problem hiding this comment.
The variable backgroundImage is referenced but not defined or imported in this file. This will cause a runtime error. Ensure backgroundImage is properly defined or imported.
| <div | |
| className="absolute inset-0" | |
| style={{ | |
| backgroundImage, | |
| backgroundSize: 'cover', | |
| backgroundPosition: 'center', | |
| backgroundRepeat: 'no-repeat', | |
| opacity: customLoaded ? 1 : 0, | |
| transition: 'opacity 0.5s ease-in-out', | |
| }} | |
| /> | |
| {/* | |
| Define backgroundImage as a valid CSS background-image value. | |
| */} | |
| {/* | |
| If you want to use a dynamic image, replace the path below with your dynamic value. | |
| */} | |
| {(() => { | |
| const backgroundImage = "url('/assets/images/hero.webp')"; | |
| return ( | |
| <div | |
| className="absolute inset-0" | |
| style={{ | |
| backgroundImage, | |
| backgroundSize: 'cover', | |
| backgroundPosition: 'center', | |
| backgroundRepeat: 'no-repeat', | |
| opacity: customLoaded ? 1 : 0, | |
| transition: 'opacity 0.5s ease-in-out', | |
| }} | |
| /> | |
| ); | |
| })()} |
There was a problem hiding this comment.
backgroundImage is defined, already used in the hero section
| backgroundSize: 'cover', | ||
| backgroundPosition: 'center', | ||
| backgroundRepeat: 'no-repeat', | ||
| opacity: customLoaded ? 1 : 0, |
There was a problem hiding this comment.
The variable customLoaded is referenced but not defined or imported in this file. This will cause a runtime error. Ensure customLoaded is properly defined or imported.
There was a problem hiding this comment.
customLoaded is a state 🤨
| const [squawkValues, setSquawkValues] = useState< | ||
| Record<string | number, string> | ||
| >({}); | ||
| const debounceTimeouts = useRef<Record<string | number, NodeJS.Timeout>>({}); |
There was a problem hiding this comment.
The debounceTimeouts ref is shared across all three debounced functions (remark, callsign, stand, squawk), which can cause race conditions when multiple fields are edited simultaneously. Each debounced function should have its own timeout ref to prevent interference.
There was a problem hiding this comment.
users can't edit multiple fields at a time
| const now = Date.now(); | ||
| const activeTime = Math.max(0, now - entry.sessionStart - (5 * 60 * 1000)); | ||
| entry.totalActive += activeTime / 60000; | ||
| const remainingActiveTime = Math.max(0, (now - entry.lastActive) / 60000 - 0.1); |
There was a problem hiding this comment.
The magic number 0.1 (6 seconds) is used without explanation. Consider defining this as a named constant (e.g., DISCONNECT_BUFFER_MINUTES) with a comment explaining why this buffer is necessary.
1ceit
left a comment
1ceit
left a comment
There was a problem hiding this comment.
This better look good or you leave me no choice....
| <div className="py-2"> | ||
| <a | ||
| href="/create" | ||
| className="block px-6 py-3 text-gray-300 hover:text-white hover:bg-blue-600/20 transition-all duration-200 font-medium" | ||
| onClick={() => setIsMenuOpen(false)} | ||
| > | ||
| Create Session | ||
| </a> | ||
| <a | ||
| href="/sessions" | ||
| className="block px-6 py-3 text-gray-300 hover:text-white hover:bg-blue-600/20 transition-all duration-200 font-medium" | ||
| onClick={() => setIsMenuOpen(false)} | ||
| > | ||
| My Sessions | ||
| </a> | ||
| <a | ||
| href="/pfatc" | ||
| className="block px-6 py-3 text-gray-300 hover:text-white hover:bg-blue-600/20 transition-all duration-200 font-medium" | ||
| onClick={() => setIsMenuOpen(false)} | ||
| > | ||
| PFATC Flights | ||
| </a> | ||
| </div> | ||
|
|
| No flight information available | ||
| </div> | ||
| )} | ||
| {searchQuery && |
| <span | ||
| id="connection-status" | ||
| className={`text-xs ${getStatusColor()}`} | ||
| > | ||
| {connectionStatus} | ||
| </span> |
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| className="gap-2" | ||
| onClick={() => navigate(`/submit/${sessionId}`)} | ||
| className="group relative flex items-center gap-2 px-4 py-2.5 rounded-xl border transition-all duration-200 bg-gradient-to-r from-zinc-500/20 to-black-500/20 border-zinc-500/40 hover:border-purple-400/60 shadow-lg shadow-purple-950/10" | ||
| > | ||
| <PlusCircle className="w-5 h-5 text-purple-400" /> | ||
| <span className="hidden sm:inline text-sm font-medium"> | ||
| New Flight | ||
| </span> | ||
| </button> | ||
| <button | ||
| onClick={handleToggleSidebar} | ||
| className={`group relative flex items-center gap-2 px-4 py-2.5 rounded-xl border transition-all duration-200 ${ | ||
| showSidebar | ||
| ? 'bg-gradient-to-r from-blue-500/20 to-blue-600/20 border-blue-500/40 hover:border-blue-400/60 shadow-lg shadow-blue-500/10' | ||
| : 'bg-gradient-to-r from-zinc-800/50 to-zinc-900/50 border-zinc-700/50 hover:border-zinc-600 hover:bg-zinc-800/70' | ||
| }`} | ||
| > | ||
| <PlusCircle className="w-5 h-5" /> | ||
| <span className="hidden sm:inline">New Flight</span> | ||
| </Button> |
There was a problem hiding this comment.
Nooooooo..... fuck your buttons. This shit better look better that I had it
No description provided.