Skip to content

Upgrading the UI to the new verawood sidebar for studio - #253

Open
felipemontoya wants to merge 3 commits into
mainfrom
fmo/verawood-ui
Open

felipemontoya wants to merge 3 commits into
mainfrom
fmo/verawood-ui

Conversation

@felipemontoya

Copy link
Copy Markdown
Member

This PR updates the UI to match the new pattern of having an icon and a sidebar that is the default in verawood.

image

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@felipemontoya
felipemontoya requested review from Henrrypg and dcoa and a lite review from Copilot September 9, 2026 23:35
@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Sep 9, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @felipemontoya!

This repository is currently maintained by @felipemontoya.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@codecov

codecov Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.45%. Comparing base (518ac25) to head (ab52325).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #253      +/-   ##
==========================================
+ Coverage   95.42%   95.45%   +0.03%     
==========================================
  Files          71       71              
  Lines        8516     8558      +42     
  Branches      451      453       +2     
==========================================
+ Hits         8126     8169      +43     
+ Misses        292      291       -1     
  Partials       98       98              
Flag Coverage Δ
unittests 95.45% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

AIUnitSidebarPanel currently drops a provided locationId and only forwards blockId, which can break context/scoping when the host passes locationId as expected by ConfigurableAIAssistance.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the Studio unit sidebar integration to match Verawood’s redesigned “icon rail + paged sidebar” pattern by wrapping the default sidebar and injecting an AI page when the host supports sidebar pages, while preserving a legacy fallback path for earlier releases.

Changes:

  • Add AIUnitSidebarPanel to extend Studio’s unit sidebar pages (or append below legacy sidebar when pages aren’t available).
  • Gate the authoring-only UnitSidebarPagesContext import behind a Tutor-version-based setting (AI_EXTENSIONS_ENABLE_UNIT_SIDEBAR_PAGE).
  • Export the new panel + defaults from the UI package, add i18n message(s), tests, and changelog entries.
File summaries
File Description
tutor/openedx_ai_extensions/plugin.py Adds Tutor-version-gated setting and changes the authoring slot contribution from Insert to Wrap to support the new sidebar layout.
tutor/openedx_ai_extensions/patches/mfe-env-config-runtime-definitions-authoring Conditionally imports Studio’s UnitSidebarPagesContext only when enabled, otherwise provides null for legacy behavior.
frontend/src/messages.ts Adds the new unit sidebar page title message descriptor.
frontend/src/index.tsx Exports AIUnitSidebarPanel, DEFAULT_UNIT_SIDEBAR_BOXES, and AISidebarBox type from the package entrypoint.
frontend/src/AIUnitSidebarPanel.tsx Implements the wrapper that adds an AI page to the sidebar pages registry, with a legacy append fallback.
frontend/src/AIUnitSidebarPanel.test.tsx Adds unit tests for both “no pages context” and “has pages context” behaviors.
frontend/package-lock.json Updates lock metadata to match the UI package version.
CHANGELOG.rst Documents the new panel, setting, and sidebar integration change in Unreleased.
Review details

Files not reviewed (1)

  • frontend/package-lock.json: Generated file

Suppressed comments (1)

frontend/src/AIUnitSidebarPanel.tsx:108

  • The panel currently maps locationId to blockId unconditionally, and the destructuring omits locationId. If the host already provides locationId, it will be dropped and ConfigurableAIAssistance will see locationId=null, which can break selector scoping. Prefer locationId ?? blockId and include both in the memo dependencies.
const AIUnitSidebarPanel = ({
  children = null,
  PagesContext = null,
  boxes = DEFAULT_UNIT_SIDEBAR_BOXES,
  icon = AutoAwesome,
  • Files reviewed: 7/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +84 to +87
courseId?: string | null;
blockId?: string | null;
unitTitle?: string | null;
readOnly?: boolean;
Comment thread frontend/src/AIUnitSidebarPanel.tsx Outdated
icon?: React.ComponentType;
pageKey?: string;
/** A react-intl MessageDescriptor; Studio's Sidebar formats it itself. */
title?: any;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

there not a good idea to loose the types, if we do really need to pass title I will suggest to update the local version of frontend platform to 8.7.1 that has i18n typed so we can do

import type { IntlShape } from '@edx/frontend-platform/i18n';

type MessageDescriptor = Parameters<IntlShape['formatMessage']>[0];
Suggested change
title?: any;
/** Formatted by Studio's Sidebar for the page heading and icon label. */
title?: MessageDescriptor;

@@ -1 +1,12 @@
const { ConfigurableAIAssistance, AIExtensionsCard } = await import("@openedx/openedx-ai-extensions-ui");
const { ConfigurableAIAssistance, AIExtensionsCard, AIUnitSidebarPanel } = await import("@openedx/openedx-ai-extensions-ui");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ConfigurableAIAssistance is imported but not used

Comment thread frontend/src/AIUnitSidebarPanel.tsx Outdated
* open, collapse and resize along with every other page.
*
* When there is no pages context to extend — an older release, or Verawood
* with ENABLE_UNIT_PAGE_NEW_DESIGN turned off, which renders the legacy

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I checked this behavior and turned off ENABLE_UNIT_PAGE_NEW_DESIGN in Verawood, however it does not insert the component in the legacy sidebar. Studio mounts UnitSidebarPagesProvider regardless of the flag (CourseUnit.tsx), so existingPages is always defined and the fallback branch never runs.

Comment thread frontend/src/AIUnitSidebarPanel.tsx Outdated
felipemontoya and others added 2 commits September 11, 2026 16:53
Co-authored-by: Diana Olarte <diana.olarte@edunext.co>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@felipemontoya

Copy link
Copy Markdown
Member Author

@dcoa thank you for the review. I have accepted the modification you made and updated the code with the rest of the comments

@dcoa

dcoa commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

thank you @felipemontoya for addressing my comments. The PR looks great. there is only one more thing I would like to address and it is user feedback:

Empty AI panel (AIUnitSidebarPanel.tsx:159): the sparkle icon shows on every unit even when no scope is configured, and opens an empty panel. The old widget hid itself.

image

To solve that I suggest to show a message that explains the unit does not have an AI tool configured (that is also the cheapest change):

Changes needed

  1. ConfigurableAIAssistance has to report "no config". Right now it returns null silently: onConfigLoad only fires when a config comes back, and onConfigError only on errors. Add an optional onNoConfig callback, called where it currently returns null. It's optional, so the learning MFE and other plugin users aren't affected.
  2. AIExtensionsSidebarPage tracks each box's state (loading / configured / none / error). It shows the empty state only when every box reports none. Waiting for all boxes avoids a flash of the empty state while they load.
  3. New i18n message for the box empty state.
  4. Tests:
    • all boxes none shows the empty state
    • one box configured shows no empty state
    • still loading shows no empty state
    • onNoConfig fires on both the 404 and the no_config responses

What the author sees

  • Sparkle icon: always on the rail, as now.
  • Opening the page: while the boxes load, a spinner, as now. Once they've all loaded:
    • If at least one box has a scope, those boxes show, as now.
    • If none do, an empty state shows instead of a blank panel. Something like: "AI Extensions is enabled" / "No AI tools are configured for this unit. Contact your platform administrator to enable one for this course or unit."
    • If loading the config fails, the box's existing error Alert shows, and no empty state.
image

@mphilbrick211 mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

5 participants