Open
Conversation
|
uma-anand
requested changes
Nov 3, 2025
Member
uma-anand
left a comment
uma-anand
left a comment
There was a problem hiding this comment.
Code quality is good. Just check of the amounts for some of the paddings etc.
| justify-content: flex-start; | ||
| align-items: stretch; | ||
| background-color: $content-card-background; | ||
| border-left: 6px solid $content-card-background; |
| display: flex; | ||
| align-items: center; | ||
| background-color: $tab-bar-background; | ||
| border-radius: 8px; |
| } | ||
| } | ||
|
|
||
| // 👇 when enableSelect is false, make all tab text white |
| align-items: center; | ||
| background-color: $tab-bar-background; | ||
| border-radius: 8px; | ||
| padding: 4px 8px; |
Member
There was a problem hiding this comment.
I feel like a lot of this isn't the actual properties in the Figma, can you double check with the designs and resubmit? Thanks
### Summary Resolves #377 ### How to Test 1. Courses already added change color to selected color palette 2. Default palette displayed for new schedule versions 3. getRandomColor works 4. compare schedule uses user's currently selected palette --------- Co-authored-by: DanielChee <128537022+DanielChee@users.noreply.github.com> Co-authored-by: Uma Anand <63426041+uma-anand@users.noreply.github.com> Co-authored-by: Uma Anand <uma2005anand@gmail.com> Co-authored-by: Alexa Fazio <45372449+afazio1@users.noreply.github.com> Co-authored-by: afazio1 <alexa.fazio04@gmail.com>
Implemented a hook that adds a comma separated list (non case sensitive) to the desired courses tab on the side. Testing: Created a button to run the hook with some test courses (not committed of course, so attaching screenshots).   
### Summary Resolves #353 <!-- What does this PR change and why? Discuss any breaking changes. --> caches seating info and removes tooltip from existing design. conditionally renders seat and waitlist info based on color of background. ### How to Test flame my code but my functionality is flawless bog --------- Co-authored-by: Uma Anand <63426041+uma-anand@users.noreply.github.com> Co-authored-by: Uma Anand <uma2005anand@gmail.com>
### Summary - wrong font color - incorrect padding ### How to Test Deploy preview --------- Co-authored-by: Uma Anand <uma2005anand@gmail.com>
### Summary Resolves #354 This is a long commit. Everything described in the issue has been implemented except for the map auto focusing on click. A few notes: 1. The design and the frontend for locations not specified are not exactly 1-1 (had already worked on this before getting access to the design - can change though). 2. Minor differences in design for the location picker component. 3. There is a 300 line variable which has all GT locations which the location search can prioritize. I could not pull everything from the crawler because some non-class location don't exist on it (e.g. Bobby Dodd). It works really good though. (have to merge this constant with the course locations constant linked in the issue). 4. To implement the location picker, our mapbox usage will double. The first API call manages the search (/suggest) , and now the second API call will extract the lat and long of the specific location the user adds (/retrieve). Screenshots <img width="1685" height="983" alt="image" src="https://github.com/user-attachments/assets/586a4e91-01c6-4ed7-b474-5edea232b330" /> <img width="1687" height="983" alt="image" src="https://github.com/user-attachments/assets/1149ad0d-84e7-4fcb-8a0f-02ed33d301b9" /> <img width="1691" height="981" alt="image" src="https://github.com/user-attachments/assets/0aa4feb6-2759-42cf-8f76-df910f4564f6" /> NYC trip appears only on zooming out manually. --------- Co-authored-by: Uma Anand <uma2005anand@gmail.com>
### Summary Resolves #364 <!-- What does this PR change and why? Discuss any breaking changes. --> Adds min travel time option to schedule combination dropdown. - Used MapBox API to generate distance array. - `GT_LOCATIONS` array has some duplicate/different names for locations. - Uses Oliver's PR's code for consistency. ### Checklist - [x] Copy over the latitudes longitudes used in Ritwic's ticket to `src/mapConstants` - [x] Use the [MapBox Matrix API](https://docs.mapbox.com/api/navigation/matrix/) once to get a matrix of distances between all the possible class locations, store this in the `src/mapConsts` file - [x] Create a new sorting option in [`src/data/beans/Oscar`](https://github.com/gt-scheduler/website/blob/594219a8b0985d9d79071558ab737e0161cbf1cd/src/data/beans/Oscar.ts#L177) to sort by the sum of time taken to go from each location to the next location on a particular day. This should automatically add it to the combination container without you having to specify it. ### How to Test <!-- Describe how to test your code. --> Tested by starting a development instance with `npm start` and using the option to create schedules. This assumes the MapBox distances are 100% accurate.
## Summary Resolves #368 This PR fixes a long-standing UX issue where the mobile navigation drawer remained open after selecting a tab. The expected behavior is that tapping a navigation item both (a) switches the active tab and (b) closes the drawer to reveal content. After this change, the drawer now closes immediately on selection. Despite early guidance that this would be a “one-line fix,” the actual implementation required a modest four-line update to correctly wire the close action alongside the tab change. The end result is simple and predictable behavior with minimal code churn. ## Background **Previous behavior:** - On mobile, opening the drawer and tapping a tab changed the active tab but left the drawer visible. - Users had to manually dismiss the drawer to view content, adding an extra step. **Desired behavior:** - Selecting a tab should immediately close the drawer so the newly selected view is visible. ## Implementation - Added the drawer close action to the same click path that handles tab changes. - Kept the change localized to the navigation event handler used by the mobile drawer flow. - No layout, styling, or rendering changes were required. - Total changes: **~4 lines** (handler composition + minor wiring). *(Yes, the “1-line fix” characterization turned out to be optimistic.)* ## Behavior Changes - **Mobile:** Selecting any navigation item now closes the drawer immediately after switching the tab. - **Desktop:** Unchanged. The drawer is not used; tab switching works as before. - **No regressions** to keyboard navigation or pointer interactions were observed. ## Rationale - Reduces friction: one tap performs the full navigation action. - Aligns with common mobile drawer patterns. - Keeps logic in one place, which is easier to maintain and reason about. ## QA / Verification Steps 1. On a mobile-sized viewport, open the drawer. 2. Tap each tab in turn: - The active tab updates. - The drawer closes immediately. 3. Reopen the drawer and select the currently active tab: - No errors; drawer still closes. 4. Switch to a desktop-sized viewport: - Drawer controls are hidden. - Tab switching continues to work normally.
### Summary Resolves #366 extend the scheduler’s active hours from 8 AM–10 PM to 6 AM–11:59 PM. OPEN and CLOSE constants in src/constants.ts were updated to 60*6 and 60*24 - 1, and the calendar height was increased from 920px to 1180px to fit the new timeframe and make sure event blocks remained the same height. Additionally fixed bug where a ghost event drag (transparent block) would remain on the screen after a quick drag-and-release. The drag logic now cancels pending animation frames so the draft always disappears immediately. ### How to Test NEW: add a 50 minute class and make sure prof name doesn't overflow (ensure event block height isnt smushed) adding recurring events (manually or dragging to create) dragging to move recurring events mobile schedule export .ics export
### Summary Resolves #356 - Modified the Oscar beans class to retrieve planned counts and store it. I had to modify the constructor setup for this, because we want the planned counts to be fetched before creating the Course beans. - Modified Course and Section beans classes to contain field `plannedCount` that stores the planned count for that course or section ### Checklist - [x] Add a method(s) in the beans/ classes to retrieve the the aggregate count information. - [x] Update Course and Section types to contain this metric. ### How to Test Deployed new cloud function to dev db and ran website locally. I used console.log() to print the counts assigned to each section/course and made sure it matched the count calculated by the function. --------- Co-authored-by: Alexa Fazio <45372449+afazio1@users.noreply.github.com>
Implemented the discard button as per the issue. <img width="1658" height="1202" alt="image" src="https://github.com/user-attachments/assets/ca15c402-4484-4dfe-880f-4cb7101c8397" /> --------- Co-authored-by: Alexa Fazio <45372449+afazio1@users.noreply.github.com>
Resolves #377 1. Courses already added change color to selected color palette 2. Default palette displayed for new schedule versions 3. getRandomColor works 4. compare schedule uses user's currently selected palette --------- Co-authored-by: DanielChee <128537022+DanielChee@users.noreply.github.com> Co-authored-by: Uma Anand <63426041+uma-anand@users.noreply.github.com> Co-authored-by: Uma Anand <uma2005anand@gmail.com> Co-authored-by: Alexa Fazio <45372449+afazio1@users.noreply.github.com> Co-authored-by: afazio1 <alexa.fazio04@gmail.com>
Implemented a hook that adds a comma separated list (non case sensitive) to the desired courses tab on the side. Testing: Created a button to run the hook with some test courses (not committed of course, so attaching screenshots).   
### Summary This PR changes the phrase "delivery mode" to be "class format" as requested by exec in the GT-Scheduler Changes for this semester. It hasn't been done yet and didn't need a ticket to be completed since it literally changes two words. Co-authored-by: Spencer Banko <spencer@lockchain.io>
This was referenced Nov 5, 2025
uma-anand
requested changes
Nov 8, 2025
Member
uma-anand
left a comment
uma-anand
left a comment
There was a problem hiding this comment.
Thanks for making the changes! Just have a couple of nits re smaller screen sizes for the TabBar
| @@ -0,0 +1,40 @@ | |||
| @import '../../variables'; | |||
|
|
|||
| .TabBar { | |||
Member
There was a problem hiding this comment.
On mobile the overflow is weird. You can take a look at the action row and make sure that this is more adaptive. Re overflow, maybe you can just hide it for now and leave a comment.
| padding: 2px; | ||
|
|
||
| .tab { | ||
| width: 100px; |
Member
There was a problem hiding this comment.
make this max width, can you check mobile and make sure it allows at least 3 tabs for mobile
Contributor
Author
There was a problem hiding this comment.
why max width? figma shows fixed width of 100
| align-items: center; | ||
| justify-content: center; | ||
| font-family: 'Roboto', sans-serif; | ||
| font-size: 14px; |
Member
There was a problem hiding this comment.
we don't use px for font size in this codebase, instead we use em
Member
|
Mobile designs are available on the Figma for your perusal |
uma-anand
previously approved these changes
Nov 10, 2025
Member
uma-anand
left a comment
uma-anand
left a comment
There was a problem hiding this comment.
Scroll is really cool! I don't want to request more changes on this so I'll take care of removing the sandbox and maybe making the fade a hook.
…on-for-enhanced-course-view-page
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #380
Adds a TabBar, CardContainer, and sandbox accessible by /sandbox/courseinfo
How to Test
start website and add /sandbox/CourseInfo to the URL