Skip to content

Replacing the header's main widget also removes the course navigation bar and masquerade bar slots #347

Open

Description

@brian-smith-tcril

Note

This issue was written by Claude (Claude Code) and reviewed before posting.

Goal: replacing the header at its top-level seam should not take the course navigation bar and the masquerade bar with it.

Why: org.openedx.frontend.slot.header.main.v1 has one default widget, org.openedx.frontend.widget.header.main.v1, the shell's Header component (registered in shell/app.ts). Header renders the <header> element with the desktop and mobile slots inside it, and then two more slots below it:

<header className="border-bottom">
  <nav>
    <SkipLink />
    <Slot id="org.openedx.frontend.slot.header.desktop.v1" />
    <Slot id="org.openedx.frontend.slot.header.mobile.v1" />
  </nav>
</header>
<Slot id="org.openedx.frontend.slot.header.masqueradeBar.v1" />
<Slot id="org.openedx.frontend.slot.header.courseNavigationBar.v1" />

A site that replaces the header with a WidgetOperationTypes.REPLACE against org.openedx.frontend.widget.header.main.v1 therefore also removes org.openedx.frontend.slot.header.masqueradeBar.v1 and, more importantly, org.openedx.frontend.slot.header.courseNavigationBar.v1: the course tabs disappear from every course page unless the replacement header renders those two slots itself. The alternative is to replace org.openedx.frontend.widget.header.desktopLayout.v1 and org.openedx.frontend.widget.header.mobileLayout.v1 separately, which is what ADR 0016 describes as replacing the header. Having to know that the one obvious seam is the wrong one, and to target two lower widgets instead, is less than ideal, and the course bar is not part of what someone means by "the header" when they bring their own.

ADR 0016 already states the rule this runs against: "A shell region that callers might want to replace wholesale belongs in its own top-level slot, with the default composition in a layout widget." The course navigation bar and the masquerade bar are regions in their own right (they appear only on routes an app opts into via provides), but they live inside the header's layout widget rather than beside it.

Open question: bump org.openedx.frontend.slot.header.main.v1 to v2?

  • For a bump: ADR 0016 treats the shell's default composition as something site configs depend on. It kept "all existing inner slot ids and default widget ids" so that plugins targeting them "continue to work without modification", and it names a REPLACE of the header's layout widget as the supported way to swap the header. A site that replaces org.openedx.frontend.widget.header.main.v1 today and re-renders the two bar slots inside its replacement would render the bars twice after this change. If the shell's default composition is contract, that is a breaking change of the slot and gets a v2: org.openedx.frontend.slot.header.main.v2 with the bar-only Header as its default widget, v1's default widget becoming a compatibility component that renders the v2 slot and the two bar slots, and v1 deprecated and later removed under ADR 0009 section 3.
  • Against a bump: ADR 0009 section 2 defines a slot's contract as its props (excluding children), where it renders and the kind of content it wraps, and says "the presence, absence, shape, or implementation of child widgets" is "explicitly not part of the slot's contract, and may change at any time". org.openedx.frontend.slot.header.main.v1 keeps all three; what changes is which component renders the two bar slots. Every slot id and default widget id stays and the default DOM order stays, so under Decide whether default widget ids and order are part of an app's contract #336 (widget existence is contract, order is not) nothing an operator can name changes. On this reading the change ships as a feat: whose release note names the double-render case.
Claude Plan

Move the two slots out of Header and up to DefaultLayout, as siblings of org.openedx.frontend.slot.header.main.v1, rendered between it and org.openedx.frontend.slot.content.main.v1:

<div className="flex-grow-0 flex-shrink-0">
  <Slot id="org.openedx.frontend.slot.header.main.v1" />
  <Slot id="org.openedx.frontend.slot.header.masqueradeBar.v1" />
  <Slot id="org.openedx.frontend.slot.header.courseNavigationBar.v1" />
</div>

Header keeps the <header> element, the skip link and the desktop and mobile slots. The slot ids and their default widget ids do not change, so existing operations against courseNavigationBar.v1, courseNavigationBar.extraContent.v1 and masqueradeBar.v1 keep working, and the default DOM order is unchanged.

Consequences to check:

  • A REPLACE of org.openedx.frontend.widget.header.main.v1 now leaves the masquerade bar and the course tabs in place. A site that wants them gone too removes org.openedx.frontend.widget.header.courseNavigationBar.v1 and org.openedx.frontend.widget.header.masqueradeBar.v1 by id, or hides them through their widget conditions.
  • isChromeVisible gates only the Header and Footer widgets today. The two bar widgets are gated by their own route-role conditions (isCourseBarRoute, isCourseBarMasqueradeRoute), so a chromeless route still shows no bars; worth a test.
  • The .header. segment in the slot ids stays accurate as a module name (ADR 0009 reads header.main.v1 as "in the header module", not "inside the header widget"), so no rename is needed.
  • ADR 0016 gets a line noting the two bars as top-level regions, or a short ADR of its own if the change is judged to need one.

Whichever answer the open question above gets, not a slot alias: an alias would route a whole-region v1 replacement onto the bar-only v2, which changes what that operation does.

No ADR placed the two slots inside Header: ADR 0016 describes Header.tsx as "a thin shell containing only top-level slots" and names only the desktop and mobile layouts as default widgets, and ADR 0013 speaks of the course bar as part of the header app, which the unchanged .header. ids keep true.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions