Evaluate centralising the JavaScript shared by styles into the common runtime
维护者通常 1 天内回复
@ignaciogros 已经在做这个了。
开始于 2026年9月13日。
评估
这个 Issue 还没有评估数据。
描述
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
- Leave as is. One
style.jsper style; rely on review discipline to keep the copies in sync. - Move the shared helpers only (
params,navExpanded,inIframe,isInViewport,checkNav) to$exeExportand have eachstyle.jscall them. Smallest change, but every style still carries the toggler wiring and its ownisLowRes(). - Move the whole menu/search toggler feature to
exe_export.js: injection of the buttons, ARIA/inertstate,nav=falsepropagation,sessionStoragepersistence (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.jsbecomes optional. - 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 withgetComputedStyle(). The breakpoint lives where the layout is defined, and the class of bugs fixed in #2389 (float/positionchecks silently taking the wrong branch) disappears.
Pros
- Single source of truth. One implementation of the
nav=falsecontract, the ARIA state and the persistence, next to the code inexe_export.jsthat 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 Vitestincludeand 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.jsand never receive accessibility fixes. With the feature in the runtime, an imported style with nostyle.jsat 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.jsdoing the same job would double-inject togglers or fight overbody.siteNav-offunless the runtime detects and defers to them (opt-in flag inconfig.xml, or "skip if#siteNavToggleralready 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 withimmutablecache headers make "new theme + oldexe_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.jsis loaded by every export, including SCORM packages where the LMS hides the navigation. The feature must stay gated onbody.exe-web-siteas 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
- Which option (1–4) and why; if 3/4, one PR or per feature (togglers → persistence → breakpoint).
- Compatibility rule for styles that still ship their own toggler code: opt-in flag in
config.xml, DOM detection, or astyle.jsAPI version. - Whether bundle/runtime versioning (the cache issue) is a prerequisite.
- Which behaviours become uniform (menu-state persistence, scroll-to-top, close-on-link-click) and which stay configurable.
- Test plan: Vitest for the runtime, one E2E spec parameterised over all bundled styles (
theme-nav-toggler-a11y.spec.tsas the seed), plus one legacy third-party style intest/fixtures/.
Out of scope
- Visual/CSS changes to any style.
- Dark mode and submenu behaviour (style-specific; may be reviewed separately).
Related
- PR #2389 (
searchBarTogglerrename,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 模板
- 没有贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
exelearning/exelearning 的其他 Issue
-
bug
难度 2/5 1-3 小时 新手友好度 77/100
exelearning/exelearning#2677 ·
维护者通常 1 天内回复
-
难度 2/5 1-3 小时 新手友好度 68/100
exelearning/exelearning#2497 ·
维护者通常 1 天内回复
-
Mermaid: long node labels are cut off in Chrome and Edge at some zoom levels, and the cut is saved in previews and exports可能已有人在做 @erseco 于 11 天前认领。 未关闭
难度 2/5 1-3 小时 新手友好度 76/100
exelearning/exelearning#2490 ·
维护者通常 1 天内回复
-
难度 2/5 1-3 小时 新手友好度 70/100
exelearning/exelearning#2455 · 1 条评论 ·
维护者通常 1 天内回复
-
non-english
难度 2/5 1-3 小时 新手友好度 64/100
exelearning/exelearning#2448 · 1 条评论 ·
维护者通常 1 天内回复
查看 exelearning/exelearning 的全部 Issue
相似的 Issue
-
FEATURE REQUEST
难度 2/5 1-3 小时 新手友好度 68/100
linuxmint/cinnamon-spices-extensions#1193 ·
维护者通常 1 天内回复
-
bug
难度 2/5 1-3 小时 新手友好度 76/100
Fei2411/DeepSeek-Bot#703 ·
-
area:web first-timers-only good first issue
难度 1/5 1 小时以内 新手友好度 95/100
career-ops-hq/career-ops#6264 ·
维护者通常 1 天内回复
-
难度 1/5 1 小时以内 新手友好度 74/100
catppuccin/firefox#21 ·
-
难度 1/5 1 小时以内 新手友好度 83/100
cline/mcp-marketplace#3394 ·