Hacktoberfest 2026:维护者为十月标记出来的 issue,仍然开放、适合新手。 浏览 Hacktoberfest issue

Evaluate centralising the JavaScript shared by styles into the common runtime

未关闭
#2,395 1 条评论 0 个 reaction 已指派 1 人 在 GitHub 查看

维护者通常 1 天内回复

@ignaciogros 已经在做这个了。

开始于 2026年9月13日。

评估

这个 Issue 还没有评估数据。

描述

enhancement

Every bundled style ships its own style.js (public/files/perm/themes/base/*/style.js, 1,373 lines across 7 files). Most of that code is not style-specific: it injects the menu/search togglers, keeps aria-expanded / aria-controls / inert in sync, propagates nav=false to the page buttons, persists the menu state and detects the responsive breakpoint. The same logic is copy-pasted per style with small drifts, and every fix has to be applied 7 times (PR #2389: the searchBarTogger typo, the isLowRes() bug in Neo/Flux and the inert fix each touched all styles).

This issue is to evaluate whether that code should move to the common runtime (public/app/common/exe_export.js, shipped with every export) so that style.js becomes optional and only holds what is genuinely specific to a style. It is an evaluation task: the outcome may be "do it", "do part of it" or "leave as is", with the reasoning recorded.

Current state

Functions present in the styles:

Function Styles Notes
toggler HTML + click handlers all 7 Educablue has no search toggler
params() (nav=false on .nav-buttons a) all 7 byte-identical
navExpanded() (aria-expanded + inert) all 7 byte-identical, added in #2389
isLowRes() all 7 three different detections: float (Base, Zen, Nova), position (Neo, Flux), matchMedia (Educablue, Universal)
inIframe() 6 identical
searchForm() 6 identical (one line)
checkNav() (fixed sidebar) Base, Neo, Nova, Zen identical
setNavOff() / persistNavState() + pre-paint guard (sessionStorage) Flux, Nova only these two remember the menu state across pages
$.fn.isInViewport most identical prototype extension, defined at global scope by each style
darkMode Educablue, Universal style-specific, different rules
dropdownMenus() Educablue, Flux, Nova, Universal similar, not identical

Genuinely style-specific today: dark mode (Educablue/Universal), collapsible submenus, the fixed-sidebar height check, and which togglers a style wants.

exe_export.js already owns the neighbouring behaviour: the search bar itself, setUrlParam(), teacherMode.withTeacherParams(), propagateNavParams(), and the search-result click handler that closes the search bar (and, since #2389, resets #searchBarToggler). The split is arbitrary: half of the nav=false contract lives in the runtime and half in each style.

Options to evaluate

  1. Leave as is. One style.js per style; rely on review discipline to keep the copies in sync.
  2. Move the shared helpers only (params, navExpanded, inIframe, isInViewport, checkNav) to $exeExport and have each style.js call them. Smallest change, but every style still carries the toggler wiring and its own isLowRes().
  3. Move the whole menu/search toggler feature to exe_export.js: injection of the buttons, ARIA/inert state, nav=false propagation, sessionStorage persistence (today Flux/Nova only — would become uniform), low-resolution behaviour (close on link click, scroll to top). The style only declares configuration (which togglers it wants, the breakpoint). style.js becomes optional.
  4. Option 3 plus a CSS-driven breakpoint: replace the three isLowRes() implementations with a custom property set by the style's CSS inside its own media query (e.g. #siteNav { --exe-nav-layout: stacked; }) read with getComputedStyle(). The breakpoint lives where the layout is defined, and the class of bugs fixed in #2389 (float/position checks silently taking the wrong branch) disappears.

Pros

  • Single source of truth. One implementation of the nav=false contract, the ARIA state and the persistence, next to the code in exe_export.js that already depends on them. A fix lands once and is covered by one unit test (exe_export.test.js) instead of 7 uncovered files (public/files/perm/themes/** is outside the Vitest include and the coverage gates).
  • Consistent UX across styles. Today only Flux and Nova remember the menu state between pages; only some styles scroll to top when opening the menu; breakpoint detection differs. Centralising makes these decisions once.
  • Third-party and user styles benefit automatically. Imported styles copy an old style.js and never receive accessibility fixes. With the feature in the runtime, an imported style with no style.js at all gets the current behaviour.
  • Smaller styles, easier authoring. A new style needs CSS, config.xml, icons and images; JavaScript only for extras. Lowers the bar for contributors and for the style review process (todo.md).
  • Testability. Runtime code is under Vitest + coverage gates; theme scripts are only exercised indirectly by E2E.

Cons / risks

  • Version coupling between styles and runtime. Today a style bundles its own logic, so an old style keeps working with a new runtime and vice versa (mostly). After centralising, old third-party styles that still ship a style.js doing the same job would double-inject togglers or fight over body.siteNav-off unless the runtime detects and defers to them (opt-in flag in config.xml, or "skip if #siteNavToggler already exists"). A compatibility rule is required and must be tested with a legacy style.
  • Cache mismatch already bites. The unversioned /api/resources/bundle/* URLs served with immutable cache headers make "new theme + old exe_export.js" a reachable combination. Moving more behaviour into the runtime widens what breaks in that case. Bundle versioning should land first or together.
  • Loss of per-style freedom. Some differences are intentional (Educablue has no search toggler; Universal scrolls to top; Neo/Zen reposition the search toggler via CSS when the menu is closed). The configuration surface must cover these, otherwise styles will reintroduce their own JS to override the runtime — back to square one.
  • Breakpoint detection. Option 4 needs every style's CSS to set the custom property; a style that forgets it is treated as always-desktop (or always-mobile). A safe default and a check (lint or E2E) are needed.
  • Bigger runtime for everyone. exe_export.js is loaded by every export, including SCORM packages where the LMS hides the navigation. The feature must stay gated on body.exe-web-site as today.
  • Migration effort. 7 bundled styles to rewrite, test/fixtures/export/** recordings to refresh, style review notes (todo.md, pr.md) to update, and an ADR (style format and export behaviour are durable decisions per AGENTS.md §7.11).
  • Documentation for style authors has to change: what the runtime does, what a style may configure, how to opt out.

Questions to answer

  1. Which option (1–4) and why; if 3/4, one PR or per feature (togglers → persistence → breakpoint).
  2. Compatibility rule for styles that still ship their own toggler code: opt-in flag in config.xml, DOM detection, or a style.js API version.
  3. Whether bundle/runtime versioning (the cache issue) is a prerequisite.
  4. Which behaviours become uniform (menu-state persistence, scroll-to-top, close-on-link-click) and which stay configurable.
  5. Test plan: Vitest for the runtime, one E2E spec parameterised over all bundled styles (theme-nav-toggler-a11y.spec.ts as the seed), plus one legacy third-party style in test/fixtures/.

Out of scope

  • Visual/CSS changes to any style.
  • Dark mode and submenu behaviour (style-specific; may be reviewed separately).

Related

  • PR #2389 (searchBarToggler rename, isLowRes() fix, aria-expanded / aria-controls / inert).
  • public/app/common/exe_export.js (setUrlParam, teacherMode, searchBar).
  • doc/architecture/adr/README.md — an ADR is expected if option 3 or 4 is chosen.

@erseco,

Could you please share your opinion on this subject before I start working on it?

Thank you.

主要语言
JavaScript
星标
145
派生
33
平均合并
3 天 14 小时
30 天内合并 PR
93

环境准备

  • 提供 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 没有贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

exelearning/exelearning 的其他 Issue

查看 exelearning/exelearning 的全部 Issue

相似的 Issue

更多 JavaScript Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。