Shared and core UI shell (migration 3/8) - #691
Conversation
Co-Authored-By: Charity Quinn <charity.quinn@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Charity Quinn <charity.quinn@cognition.ai>
| return ( | ||
| <header> | ||
| <div id="header"> | ||
| <NavLink className="home-link" to="/news/1" onClick={scrollTop}> |
There was a problem hiding this comment.
📝 Info: NavLink active class relies on v6 string-append behavior
NavLink with a static string className="home-link" reproduces Angular's routerLinkActive="active" only because react-router-dom v6 appends active to string classNames when active. Confirmed against ^6.26.2 in package.json, so the port is faithful.
Was this helpful? React with 👍 or 👎 to provide feedback.
| name="theme" | ||
| type="number" | ||
| onChange={changeTitleFont} | ||
| onKeyUp={changeTitleFont} | ||
| /> | ||
| </label> | ||
| </div> | ||
| <div> | ||
| <label> | ||
| List spacing: | ||
| <input | ||
| min="0" | ||
| value={settings.listSpacing} | ||
| name="theme" |
There was a problem hiding this comment.
📝 Info: Number inputs carry stray name="theme"
The font-size and list-spacing number inputs both keep name="theme", copied verbatim from the Angular template. The attribute is inert for number inputs in React, so behavior is unaffected, but the naming is misleading.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verbatim from the Angular template, and inert as you say — leaving it so the markup diffs cleanly against the original. Worth a cleanup pass once the stack lands.
| onChange={() => setTheme('default')} | ||
| /> | ||
| {' Default'} | ||
| </label> | ||
| </div> | ||
| <div> | ||
| <label> | ||
| <input | ||
| name="theme" | ||
| type="radio" | ||
| value="night" | ||
| checked={settings.theme === 'night'} | ||
| onChange={() => setTheme('night')} | ||
| /> | ||
| {' Night'} | ||
| </label> | ||
| </div> | ||
| <div> | ||
| <label> | ||
| <input | ||
| name="theme" | ||
| type="radio" | ||
| value="amoledblack" | ||
| checked={settings.theme === 'amoledblack'} | ||
| onChange={() => setTheme('amoledblack')} |
There was a problem hiding this comment.
📝 Info: Theme radios switched from click to change
Angular selected a theme on (click); the port uses onChange on the controlled radios. This is idiomatic for React and also fires on keyboard selection, so there is no user-facing regression.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Stacked on #690. Ports the app shell —
loader,error-message,header,footer,settings— as function components, plus the global stylesheets. The router outlet is a placeholder until PR4 adds the feeds..scssverbatim, andsrc/styles.scss+ the_media/_themes/_theme_variablespartials are copied undersrc-react/./assets/images/logo.svg) because Vite servespublic/at the root, where Angular resolved them relative tobase href="/".NavLinkfor the feed tabs (routerLinkActive->activeviaNavLink's default) and keeps the scroll-to-top-on-click behavior.settings.componentis wired touseSettings()from PR2; the inputs stay controlled and the font/spacing fields keep their key-up semantics, so behavior is identical to the Angular[(ngModel)]+(keyup)pair. The theme radios useonChangerather thanonClick— withcheckedset,onClickalone makes React treat them as read-only controlled inputs and warn.styles.scss.app-loader/#root:empty + .app-loaderrules are dropped rather than carried over: Angular'sindex.htmlheld that spinner markup and the framework replaced the host element's content on boot, whereas React mounts into the container, so an:empty-driven overlay would never be hidden again.App.tsxbecomes the shell fromapp.component.htmlwith the theme class bound tosettings.theme;main.tsxwraps it inBrowserRouter+SettingsProvider.No
dangerouslySetInnerHTMLhere — the only[innerHTML]bindings are API-provided comment/about HTML, which arrive in PR5/PR6.Verified:
yarn react:buildpasses; header/footer/settings render and theme + font-size + spacing changes apply and persist across reload, with no controlled-input warning in the console.Devin-Org: engineering
Link to Devin session: https://app.devin.ai/sessions/1ff25c6cf2f949458f82cc596fc79c65
Requested by: @charityquinn-cognition
Devin Review