Skip to content

Update profile dropdown menu#2953

Open
ascholerChemeketa wants to merge 24 commits into
PreTeXtBook:masterfrom
ascholerChemeketa:profile-dropdown-accessibility
Open

Update profile dropdown menu#2953
ascholerChemeketa wants to merge 24 commits into
PreTeXtBook:masterfrom
ascholerChemeketa:profile-dropdown-accessibility

Conversation

@ascholerChemeketa

Copy link
Copy Markdown
Contributor

Adds PTXDropdown framework and uses that to make Profile menu accessible.
Gets rid of hard coded RS menu items.

This builds on #2952

Starting at f46495a

After that PR is in I can rebase to that point.

@ascholerChemeketa ascholerChemeketa force-pushed the profile-dropdown-accessibility branch from d1b920c to f14c749 Compare June 20, 2026 18:20
@rbeezer

rbeezer commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

I think this must need a rebase, and I suspect it will be of a bigger magnitude than I should be touching? Along the way, Claude suspects one "dormant" bug and has some nits. Coming next.

@rbeezer

rbeezer commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

The PTXDropdown approach is the right move — proper menu semantics and keyboard handling, and it fixes the invalid <div>-inside-<button> nesting the old profile menu had. A few things.

One actual bug:

  • In the PTXDropdown constructor, the "close on escape" listener dereferences this.controlElement outside the if (this.controlElement) guard that wraps the other control-element listeners:

    // Close on escape even if opened via click and focus is on button
    this.controlElement.addEventListener("keydown", (event) => {
    if (event.key === "Escape" && this.isOpen()) {

    Since openButton defaults to null and you use this.controlElement?.contains(...) elsewhere, the class is meant to tolerate a null control — so new PTXDropdown(menuEl) would throw here. The current call site passes a button, so it doesn't bite today. Folding this listener into the existing if (this.controlElement) block fixes the crash and also drops a duplicate Escape path (it's already handled in handleKeydown). That inner if is also mis-indented.

Smaller:

  • console.log("PTXDropdown: No dropdown element provided.") reads more like a console.warn.
  • pretext-dropdown.js has no trailing newline.
  • XSL: aria-labelledby="ptx-user-dropdown-button" role="menu" has a double space; and aria-label="Runestone" is the one hardcoded string in the new menu (probably fine as a product name).
  • Worth a visual check: the old ptx-dropdown-button mixin set position: relative on the button, and the renamed ptx-dropdown-menu mixin (now on the menu element) uses position: absolute; top: 100%; right: 0 without that anchor — confirm the menu still positions under the button rather than against a higher navbar ancestor.

Claude Opus 4.8, acting as a review assistant for Rob Beezer

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants