-
Notifications
You must be signed in to change notification settings - Fork 1
DON'T MERGE First approach for embeddable version #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98bd9246ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Priority 3: No server URL configured | ||
| if (!this.eXeLearning.config.fullURL) { | ||
| return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Treat root installs as server mode
The static-mode heuristic treats an empty config.fullURL as “no server configured”, but the server sets fullURL to basePath, which is an empty string for root installs (BASE_PATH unset). In that common deployment, this branch forces static mode even though a server is available, so API routes won’t load and the app will fall back to empty static data. That breaks normal server behavior for root installs; consider basing this on an explicit static flag/file protocol or a separate server indicator rather than fullURL being non-empty.
Useful? React with 👍 / 👎.
This pull request introduces a new unified data access layer to support a static/offline mode for the eXeLearning app, allowing it to run without a backend server. It adds the
DataProviderabstraction, updates the app initialization and runtime logic to detect and operate in static mode, and adapts build and deployment workflows for static site generation and PR previews. The changes also include updates to tests and deployment scripts to ensure compatibility with the new static mode.Core application changes:
DataProviderclass (public/app/core/DataProvider.js) that abstracts data access, switching between server API calls and pre-bundled static data depending on the detected mode. This enables the app to function both online and offline.Appclass (public/app/app.js) to initialize and useDataProvider, detect static/offline mode via multiple signals (explicit flag, file protocol, or missing server URL), and adjust flows such as checks and modals accordingly. [1] [2] [3] [4] [5] [6]public/app/common/common.js)Build and deployment workflow updates:
build:staticto generate a static bundle for offline use (package.json)..github/workflows/pr-preview.yml) to build and deploy PR previews as static sites, leveraging the static build output..github/workflows/docs-and-repos.yml) to use a unified deploy action and adjusted permissions for static site deployment. [1] [2]Testing and configuration:
public/app/app.test.jsto mock static mode detection and ensure normal flows are tested for both modes. [1] [2] [3] [4]These changes lay the foundation for fully supporting offline/static usage of the app, improving flexibility for users who need to run eXeLearning without a backend server.