feat: add welcome route#18
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a temporary /welcome route to the learner dashboard app to render the Progressive Profiling page (intended to match the welcome page behavior from frontend-app-authn) for local testing and transitional use.
Changes:
- Add a new
/welcomeroute renderingProgressiveProfiling. - Add a local development env var for the Progressive Profiling support link.
- Minor JSON formatting normalization in
package.json/package-lock.json.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/index.jsx | Adds ProgressiveProfiling import and wires a new welcome route into the top-level router. |
| package.json | Formatting-only change (no dependency changes included). |
| package-lock.json | Formatting-only change (no dependency changes included). |
| .env.development | Adds a dev-only support-link env var for progressive profiling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| subscribe, | ||
| mergeConfig, | ||
| } from '@edx/frontend-platform'; | ||
| import { ProgressiveProfiling } from '@edx/frontend-plugin-learner-dashboard'; |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| subscribe, | ||
| mergeConfig, | ||
| } from '@edx/frontend-platform'; | ||
| import { ProgressiveProfiling } from '@edx/frontend-plugin-learner-dashboard'; |
| "@edx/frontend-component-header": "^8.0.0", | ||
| "@edx/frontend-enterprise-hotjar": "7.2.0", | ||
| "@edx/frontend-platform": "^8.3.1", | ||
| "@edx/frontend-plugin-learner-dashboard": "^3.3.5", |
There was a problem hiding this comment.
any reason we're now adding Frontend Plugin Learner Dashboard as a dependency here instead of continuing to update the version via edx-internal?
There was a problem hiding this comment.
This is just to make sure the linter is happy. We could tell eslint to ignore the fact that frontend-plugin-learner-dashboard is not installed. But I was worried something may get missed.
I believe we still overwrite the version in the build step so it's pretty much the same thing. We will also need the plugins for https://2u-internal.atlassian.net/browse/SUBS-663 so I figured may as well add in the library as a "real" dependency.
There was a problem hiding this comment.
Personally, just so we don't lead to confusion about where the true version upgrade is coming from, I'd include the dependency in only one place. Fair point on needing it for future work/development. If this is the case, and knowing we'll be moving to frontend-base, maybe we can have a ticket to remove the dependency from edx-internal?
| <AppProvider store={store}> | ||
| <Routes> | ||
| <Route path="/" element={<PageWrap><App /></PageWrap>} /> | ||
| <Route path="welcome" element={<PageWrap><ProgressiveProfiling /></PageWrap>} /> |
There was a problem hiding this comment.
nit: don't we usually use the forward slash like /welcome for these routes?
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Adds the temporary
/welcomeroute for the duplicated welcome page. This is meant to be identical to the welcome page in frontend-app-authn.We will be removing this code once we switch over to the modal based Progressive Profiling approach.
To test:
mfrank/add-replica-progressive-profilingbranch fromfrontend-plugin-learner-dashboardmodule.config.jsnpm ci && npm startlocalhost:1996/welcome