Release 0.179.1 #3521
Release 0.179.1 #3521
Conversation
* fix(deps): update dependency jsdom to v26 * fix: add window proxy to update location * docs: documenting reason for proxy --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: asajjad2 <areebsajjad21@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* feat: add drawer * style: fix mobile-nav styling * style: cleaning up styles, precise values * style: fix buttons * style: topics capitalized * tests: update to check drawer toggles * fix: mobile menu below nav * style: button alignment * fix: user menu added in mobile nav * fix: menu button shouldn't show on desktop * fix: update user menu logic on mobile * fix: useEffect for managing drawer state * refactor: repeated code removed * fix: aria-label * refactor: auth buttons repitition removed * tests: update test to reflect nav changes * fix: hide slider buttons from nav drawer * refactor: auth buttons and test * refactor: extract components to reduce TopAppBar complexity * fix: mobile user menu z-index to appear above drawer * test: fix TopAppBar tests after component refactoring * feat: close mobile drawer when auth buttons are clicked * feat: close mobile drawer when usermenu opens * refactor: remove data-bs-toggle not available on bs-4 * refactor: put back .navbar-collapse.show.collapsing css
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @odlbot, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! I'm gemini-code-assist, an experienced software engineer here to provide a summary of this pull request. This PR appears to be a release branch for version 0.179.1. It primarily incorporates several dependency updates, many of which were likely automated by Renovate bot, along with a specific fix related to the mobile navigation menu. The changes involve updating version strings, bumping various Python and Node.js package versions (including significant major version updates for some frontend libraries like jsdom and js-yaml, and backend libraries like django-ipware and mitol-django-common), and implementing the necessary code changes to support the mobile navigation update.
Highlights
- Dependency Updates: Multiple Python and Node.js dependencies have been updated, including major version bumps for
jsdom(v16 to v26),js-yaml(v3 to v4),django-ipware(v3 to v7),django-redis(v5.0 to v5.4),mitol-django-common(2023 to 2025), andmitol-django-olposthog(2024 to 2025). This includes updates topackage.json,pyproject.toml, and the correspondingyarn.lockandpoetry.lockfiles. - Mobile Navigation Rework: The top application bar (
TopAppBar) and catalog menu (CatalogMenu) components have been refactored to implement a new mobile drawer navigation pattern. This includes adding state management for the drawer, new components likeAuthButtonsfor mobile sign-in/create account links, and associated CSS styles. - JSDOM v26 Compatibility: A specific fix was implemented in
static/js/babelhook.jsto handle changes in JSDOM v26 regardingwindow.locationassignments, now usingjsdom.reconfigurevia a Proxy. - Version String Update: The project version string in
mitxpro/settings.pyhas been updated to0.179.1.
Changelog
Click here to see the changelog
- RELEASE.rst
- Added release notes section for version 0.179.1, listing included changes.
- mitxpro/settings.py
- Updated the
VERSIONconstant to '0.179.1'.
- Updated the
- package.json
- Updated
jsdomdependency from ^16.2.2 to ^26.0.0. - Updated
js-yamldependency from ^3.13.1 to ^4.0.0.
- Updated
- poetry.lock
- Updated locked versions for numerous Python dependencies, including major versions for
django-ipware,django-redis,mitol-django-common, andmitol-django-olposthog. - Added
python-ipwareanddjango-stubsas new transitive dependencies. - Removed several old transitive dependencies.
- Updated locked versions for numerous Python dependencies, including major versions for
- pyproject.toml
- Updated
django-ipwarefrom 3.0.7 to 7.0.1. - Updated
django-redisfrom 5.0.0 to 5.4.0. - Updated
mitol-django-commonfrom 2023.12.19 to 2025.5.23. - Updated
mitol-django-olposthogfrom ^2024.5.14 to ^2025.0.0.
- Updated
- static/js/babelhook.js
- Implemented a Proxy to intercept
window.locationassignments and usejsdom.reconfigurefor compatibility with JSDOM v26. - Removed the old
Object.definePropertyapproach forwindow.location.
- Implemented a Proxy to intercept
- static/js/components/CatalogMenu.js
- Added
isMobileprop to conditionally render different markup for mobile navigation. - Created
renderMenuItemshelper function to avoid code duplication. - Removed
data-bs-toggle="dropdown"from the main link when not in mobile mode.
- Added
- static/js/components/CatalogMenu_test.js
- Added tests to verify mobile rendering logic and class names.
- Added tests to ensure dropdown elements are not rendered in mobile mode.
- static/js/components/TopAppBar.js
- Refactored into a functional component using
useStateanduseEffectfor mobile drawer state. - Added
toggleDrawerfunction anddrawerOpenstate. - Used
useEffectto manage body overflow when the drawer is open. - Introduced
EXCLUDED_LOGIN_PATHSconstant. - Created
renderMobileControls,renderDesktopNav, andrenderMobileDrawerhelper functions. - Integrated the new
AuthButtonscomponent. - Passed
isMobile={true}toCatalogMenuwithin the mobile drawer.
- Refactored into a functional component using
- static/js/components/TopAppBar_test.js
- Updated tests to reflect the use of the
AuthButtonscomponent. - Added tests for mobile drawer functionality (toggling,
isMobileprop passing toCatalogMenu).
- Updated tests to reflect the use of the
- static/js/components/UserMenu.js
- Added an optional
onMouseDownprop to the user menu link. - Removed
data-bs-toggle="dropdown"from the user menu link.
- Added an optional
- static/js/components/input/AuthButtons.js
- Added a new React component
AuthButtonsto render Sign In and Create Account links. - Added
isMobileprop to conditionally apply mobile-specific styling/structure.
- Added a new React component
- static/js/components/input/AuthButtons_test.js
- Added tests for the
AuthButtonscomponent to verify links.
- Added tests for the
- static/scss/detail/header.scss
- Increased the
z-indexof the#headerelement from 1 to 100.
- Increased the
- static/scss/top-app-bar.scss
- Added extensive new CSS rules to style the mobile navigation drawer, overlay, header, close button, auth buttons, catalog menu, and user menu in mobile view.
- yarn.lock
- Updated locked versions for numerous Node.js dependencies, reflecting changes in
package.jsonand transitive dependencies. - Removed several old transitive dependencies.
- Added several new transitive dependencies (
@asamuzakjp/css-color,@csstools/color-helpers,@csstools/css-calc,@csstools/css-color-parser,entities,rrweb-cssom,tldts-core,tldts).
- Updated locked versions for numerous Node.js dependencies, reflecting changes in
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Dependencies abound,
New versions all around.
Mobile menu slides,
Code review guides.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces version 0.179.1, primarily focusing on dependency updates and a functional improvement to the mobile navigation. The changes appear well-structured and address the necessary updates for the dependencies, including handling a breaking change in jsdom. The implementation of the mobile navigation drawer seems clean and follows standard React practices. No specific style guide was provided for this review, so feedback is based on commonly accepted best practices for Python, JavaScript, and CSS.
Summary of Findings
- Dependency Updates: The pull request includes several dependency updates, including major version bumps for
jsdom,django-ipware,mitol-django-common, andmitol-django-olposthog. The code changes inbabelhook.jscorrectly address a breaking change injsdomv26. - Mobile Navigation Implementation: The implementation of the mobile navigation drawer in
TopAppBar.jsandCatalogMenu.js, along with the newAuthButtons.jscomponent and associated styles, is well-structured and uses standard React patterns (state, effects, component composition). - Code Organization: The refactoring of rendering logic into separate functions (
renderMenuItems,renderMobileControls, etc.) improves the readability and maintainability of theTopAppBarandCatalogMenucomponents. - Minor Code Style/Optimization: Some minor opportunities for code style consistency or slight CSS optimization were observed but not commented on directly, as per the review settings (only commenting on medium+ severity issues).
Merge Readiness
Based on the review of the provided code changes, the pull request appears to be in good shape for merging. The dependency updates are necessary, and the mobile navigation feature is implemented cleanly using standard practices. No critical or high severity issues were found. Please ensure that the dependency updates have been tested thoroughly in a staging environment. As a code reviewer, I am unable to directly approve this pull request, but I recommend that other reviewers examine the changes and approve if they agree with this assessment.
| // Proxy window.location assignments to use jsdom.reconfigure() | ||
| // instead of direct assignment, which is no longer allowed in v26 | ||
| const windowProxy = new Proxy(window, { | ||
| set: function (target, prop, value) { | ||
| if (prop === "location") { | ||
| let url = value; | ||
| if (!url.startsWith("http")) { | ||
| url = `http://fake${url}`; | ||
| } | ||
| jsdom.reconfigure({ url }); | ||
| return true; | ||
| } | ||
|
|
||
| return Reflect.set(target, prop, value); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
This proxy for window.location is a good solution to handle the breaking change in jsdom v26 regarding direct assignment. It correctly intercepts assignments and uses jsdom.reconfigure.
| const CatalogMenu = ({ courseTopics, isMobile = false }: Props) => { | ||
| const renderMenuItems = () => ( | ||
| <> | ||
| <a | ||
| className={ | ||
| isMobile ? "mobile-catalog-item all-topics" : "dropdown-item bold" | ||
| } | ||
| href="/catalog/" | ||
| aria-label="All Topics" | ||
| > | ||
| All Topics | ||
| </a> | ||
| {courseTopics?.map((courseTopic, index) => ( | ||
| <a | ||
| className={isMobile ? "mobile-catalog-item" : "dropdown-item"} | ||
| key={index} | ||
| href={`/catalog/?topic=${encodeURIComponent(courseTopic.name)}`} | ||
| aria-label={courseTopic.name} | ||
| > | ||
| {courseTopic.name} ({courseTopic.course_count || 0}) | ||
| </a> | ||
| ))} | ||
| </> | ||
| ); | ||
|
|
||
| if (isMobile) { | ||
| return ( | ||
| <div className="mobile-drawer-section"> | ||
| <a | ||
| className="mobile-drawer-heading" | ||
| href="/catalog/" | ||
| aria-label="Courses" | ||
| > | ||
| Courses | ||
| </a> | ||
| <div className="mobile-catalog-menu">{renderMenuItems()}</div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Refactoring the menu item rendering into a separate function renderMenuItems is a good approach to avoid code duplication between the mobile and desktop views. The conditional rendering based on the isMobile prop is clear.
| const [drawerOpen, setDrawerOpen] = useState(false); | ||
| const toggleDrawer = () => { | ||
| setDrawerOpen((prev) => !prev); | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| document.body.style.overflow = drawerOpen ? "hidden" : ""; | ||
| return () => { | ||
| document.body.style.overflow = ""; | ||
| }; | ||
| }, [drawerOpen]); |
There was a problem hiding this comment.
Using useState for drawerOpen and useEffect to manage the document.body.style.overflow is a standard and effective way to handle the mobile drawer state and prevent body scrolling when it's open.
| const renderMobileControls = () => ( | ||
| <div className="d-flex align-items-center d-md-none"> | ||
| <button | ||
| className="navbar-toggler nav-opener d-flex align-items-center" | ||
| type="button" | ||
| onClick={toggleDrawer} | ||
| aria-controls="nav" | ||
| aria-expanded={drawerOpen} | ||
| aria-label="Toggle navigation" | ||
| > | ||
| <span className="navbar-toggler-icon" /> | ||
| Menu | ||
| </button> | ||
|
|
||
| {showLoginSignup && isAuthenticated && ( | ||
| <div className="mobile-user-menu"> | ||
| <UserMenu | ||
| currentUser={currentUser} | ||
| onMouseDown={() => drawerOpen && toggleDrawer()} | ||
| /> | ||
| </a> | ||
| {errorPageHeader ? null : ( | ||
| <button | ||
| className="navbar-toggler nav-opener" | ||
| type="button" | ||
| data-toggle="collapse" | ||
| data-target="#nav" | ||
| aria-controls="nav" | ||
| aria-expanded="false" | ||
| aria-label="Toggle navigation" | ||
| > | ||
| <span className="navbar-toggler-icon" /> | ||
| Menu | ||
| </button> | ||
| )} | ||
| {errorPageHeader ? null : ( | ||
| <ul | ||
| id="nav" | ||
| className="collapse navbar-collapse px-0 justify-content-end" | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Separating the mobile controls into renderMobileControls improves the readability and organization of the component.
| const renderDesktopNav = () => ( | ||
| <ul | ||
| id="nav" | ||
| className="navbar-collapse d-none d-md-flex px-0 justify-content-end" | ||
| > | ||
| <li> | ||
| <CatalogMenu courseTopics={courseTopics} /> | ||
| </li> | ||
| {SETTINGS.enable_enterprise && ( | ||
| <li> | ||
| <a | ||
| href={routes.enterprise} | ||
| className="enterprise-link" | ||
| aria-label="enterprise" | ||
| > | ||
| <li> | ||
| <CatalogMenu courseTopics={courseTopics} /> | ||
| </li> | ||
| {SETTINGS.enable_enterprise ? ( | ||
| <li> | ||
| <a | ||
| href={routes.enterprise} | ||
| className="enterprise-link" | ||
| aria-label="enterprise" | ||
| > | ||
| Enterprise | ||
| </a> | ||
| </li> | ||
| ) : null} | ||
| <li> | ||
| <a | ||
| href={routes.webinars} | ||
| className="webinar-link" | ||
| aria-label="webinars" | ||
| > | ||
| Webinars | ||
| </a> | ||
| </li> | ||
| <li> | ||
| <a href={routes.blog} className="blog-link" aria-label="blog"> | ||
| Blog | ||
| </a> | ||
| </li> | ||
| {shouldShowLoginSignup(location) ? ( | ||
| currentUser && currentUser.is_authenticated ? ( | ||
| <li> | ||
| <UserMenu currentUser={currentUser} /> | ||
| </li> | ||
| ) : ( | ||
| <React.Fragment> | ||
| <li> | ||
| <MixedLink | ||
| dest={routes.login.begin} | ||
| className="button" | ||
| aria-label="Login" | ||
| > | ||
| Sign In | ||
| </MixedLink> | ||
| </li> | ||
| <li> | ||
| <MixedLink | ||
| dest={routes.register.begin} | ||
| className="button" | ||
| aria-label="Login" | ||
| > | ||
| Create Account | ||
| </MixedLink> | ||
| </li> | ||
| </React.Fragment> | ||
| ) | ||
| ) : null} | ||
| </ul> | ||
| Enterprise | ||
| </a> | ||
| </li> | ||
| )} | ||
| <li> | ||
| <a | ||
| href={routes.webinars} | ||
| className="webinar-link" | ||
| aria-label="webinars" | ||
| > | ||
| Webinars | ||
| </a> | ||
| </li> | ||
| <li> | ||
| <a href={routes.blog} className="blog-link" aria-label="blog"> | ||
| Blog | ||
| </a> | ||
| </li> | ||
| {showLoginSignup && | ||
| (currentUser && currentUser.is_authenticated ? ( | ||
| <li> | ||
| <UserMenu currentUser={currentUser} /> | ||
| </li> | ||
| ) : ( | ||
| <AuthButtons /> | ||
| ))} | ||
| </ul> | ||
| ); |
There was a problem hiding this comment.
Separating the desktop navigation into renderDesktopNav improves the readability and organization of the component.
| {drawerOpen && ( | ||
| <div | ||
| className="drawer-overlay d-md-none" | ||
| onClick={toggleDrawer} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Adding the drawer-overlay is important for accessibility and user experience, providing a visual cue and click target to close the drawer.
| type Props = { | ||
| /* This is here for future use when we have custom profile avatars */ | ||
| currentUser: User, | ||
| onMouseDown?: () => void, | ||
| }; |
There was a problem hiding this comment.
Adding the optional onMouseDown prop allows the parent component (TopAppBar) to trigger the drawer closing when the user menu is clicked in mobile view. This is a good pattern for coordinating state between components.
| // @flow | ||
| import React from "react"; | ||
|
|
||
| import { routes } from "../../lib/urls"; | ||
| import MixedLink from "../MixedLink"; | ||
|
|
||
| type Props = { | ||
| isMobile?: boolean, | ||
| onClick?: () => void, | ||
| }; | ||
|
|
||
| const AuthButtons = ({ isMobile = false, onClick }: Props) => { | ||
| const authLinks = [ | ||
| { | ||
| key: "login", | ||
| label: "Sign In", | ||
| dest: routes.login.begin, | ||
| ariaLabel: "Login", | ||
| }, | ||
| { | ||
| key: "create-account", | ||
| label: "Create Account", | ||
| dest: routes.register.begin, | ||
| ariaLabel: "Create Account", | ||
| }, | ||
| ]; | ||
|
|
||
| const className = isMobile ? "mobile-auth-button" : "button"; | ||
|
|
||
| const buttons = authLinks.map(({ key, label, dest, ariaLabel }) => ( | ||
| <MixedLink | ||
| key={key} | ||
| dest={dest} | ||
| className={className} | ||
| aria-label={ariaLabel} | ||
| onClick={onClick} | ||
| > | ||
| {label} | ||
| </MixedLink> | ||
| )); | ||
|
|
||
| return isMobile ? ( | ||
| <div className="mobile-auth-buttons">{buttons}</div> | ||
| ) : ( | ||
| <> | ||
| {buttons.map((button) => ( | ||
| <li key={button.key}>{button}</li> | ||
| ))} | ||
| </> | ||
| ); | ||
| }; | ||
|
|
||
| export default AuthButtons; |
There was a problem hiding this comment.
Creating a dedicated AuthButtons component is a good refactoring step, encapsulating the logic for rendering login/signup links and handling mobile/desktop variations.
| background-color: white; | ||
| position: relative; | ||
| z-index: 1; | ||
| z-index: 100; |
There was a problem hiding this comment.
Increasing the z-index of the header is necessary to ensure it stays above the new mobile drawer overlay.
| .mobile-drawer { | ||
| position: fixed; | ||
| top: 98px; | ||
| right: -366px; | ||
| width: 366px; | ||
| height: calc(100vh - 98px); | ||
| background-color: white; | ||
| transition: right 0.3s ease; | ||
| z-index: 1050; | ||
| overflow-y: auto; | ||
| padding: 26px 32px; | ||
| box-shadow: -2px 0 10px rgb(0 0 0 / 10%); | ||
|
|
||
| &.open { | ||
| right: 0; | ||
| } | ||
| } | ||
|
|
||
| .drawer-overlay { | ||
| position: fixed; | ||
| top: 98px; | ||
| left: 0; | ||
| width: 100%; | ||
| height: calc(100% - 98px); | ||
| background-color: rgb(0 0 0 / 50%); | ||
| z-index: 1040; | ||
| } | ||
|
|
||
| .drawer-header { | ||
| display: flex; | ||
| justify-content: flex-end; | ||
| margin-bottom: 1rem; | ||
| } | ||
|
|
||
| .close-drawer { | ||
| background: none; | ||
| border: none; | ||
| font-size: 1.75rem; | ||
| line-height: 1; | ||
| padding: 0; | ||
| cursor: pointer; | ||
| } | ||
|
|
||
| .mobile-auth-buttons { | ||
| padding: 8px 0; | ||
| display: flex; | ||
| gap: 12px; | ||
| } | ||
|
|
||
| a.mobile-auth-button { | ||
| flex: 1; | ||
| justify-content: center; | ||
| align-items: center; | ||
| text-align: center; | ||
| background: $primary; | ||
| color: white; | ||
| padding: 6px 10px; | ||
|
|
||
| &:hover, | ||
| &:focus { | ||
| background: black; | ||
| color: white; | ||
| text-decoration: none; | ||
| } | ||
| } | ||
|
|
||
| .mobile-catalog-menu { | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 8px; | ||
| padding-left: 12px; | ||
| } | ||
|
|
||
| .mobile-catalog-item { | ||
| display: block; | ||
| color: #333; | ||
| text-decoration: none; | ||
| text-transform: capitalize; | ||
| font-size: 0.9375rem; | ||
| line-height: 1.4; | ||
|
|
||
| &:hover, | ||
| &:focus { | ||
| color: $primary; | ||
| text-decoration: none; | ||
| } | ||
| } | ||
|
|
||
| .mobile-drawer-heading { | ||
| padding: 4px 0; | ||
| color: black; | ||
| font-weight: 700; | ||
| } | ||
|
|
||
| .mobile-drawer-section { | ||
| padding: 8px 0; | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 8px; | ||
| } | ||
|
|
||
| .mobile-user-menu { | ||
| margin-left: 10px; | ||
| z-index: 1060; | ||
| } |
There was a problem hiding this comment.
The new SCSS rules for .mobile-drawer, .drawer-overlay, and related elements provide the necessary styling for the mobile navigation drawer. The use of position: fixed, z-index, and transition is appropriate for this pattern.
odlbot
added
deploying to prod
deployed to prod
and removed
all checkboxes checked
deploying to prod
labels
renovate[bot]
Areeb Sajjad