New Vaadin UI integration#2377
Conversation
There was a problem hiding this comment.
Pull request overview
This PR integrates Vaadin web components as a new UI component library for OpenRemote, creating a migration path away from the existing Material Web Components (MWC). Key changes include introducing new @openremote/or-vaadin-components and @openremote/theme packages, updating build tooling with newer rspack/rsbuild versions, and beginning the migration of input components.
Key Changes:
- New Vaadin component wrappers (textfield, textarea, numberfield, checkbox, select, passwordfield)
- New unified theme system with CSS custom properties supporting light/dark modes
- Migration of attribute input system to use Vaadin components
- Deprecation markers added to or-mwc-input
- Removal of or-data-viewer from manager app
Reviewed changes
Copilot reviewed 40 out of 42 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/component/or-vaadin-components/* | New package wrapping Vaadin components with OpenRemote-specific behavior |
| ui/component/theme/* | New theme package defining CSS custom properties for design system |
| ui/component/or-attribute-input/src/index.ts | Updated to use new Vaadin input provider |
| ui/component/or-mwc-components/src/or-mwc-input.ts | Added deprecation notices and moved logic to or-vaadin-components |
| ui/app/manager/src/index.ts | Theme integration in manager app |
| yarn.lock | Dependency updates including rspack 1.6.4, rsbuild 1.6.8, and Vaadin 25.0.0-rc1 |
| ui/app/manager/src/pages/page-insights.ts | Removed or-data-viewer integration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .filter(attr => !OrVaadinInput.elementProperties.has(attr)) | ||
| .forEach(attr => OrVaadinInput.createProperty(attr, constr.getPropertyOptions(attr))) |
There was a problem hiding this comment.
The registerProperties function references OrVaadinInput.elementProperties and OrVaadinInput.createProperty but these properties/methods are not defined in the OrVaadinInput class. This will cause a runtime error when the function is called at line 273.
| } | ||
|
|
||
| // Before the class is initialized, register the property fields of the underlying Vaadin classes | ||
| registerProperties(OrVaadinInput.VAADIN_CLASSES); |
There was a problem hiding this comment.
The registerProperties function is called at line 273 before the OrVaadinInput class is fully initialized. If the function relies on static properties or methods from OrVaadinInput, this may cause initialization issues. Consider moving this call inside the class as a static initializer block or ensure all dependencies are available at this point.
| } | |
| // Before the class is initialized, register the property fields of the underlying Vaadin classes | |
| registerProperties(OrVaadinInput.VAADIN_CLASSES); | |
| static { | |
| // After the class is fully initialized, register the property fields of the underlying Vaadin classes | |
| registerProperties(this.VAADIN_CLASSES); | |
| } | |
| } |
| dataViewer?: any // deprecated DataViewerConfig | ||
| } | ||
|
|
||
| export function pageInsightsProvider(store: Store<AppStateKeyed>, config?: PageInsightsConfig): PageProvider<AppStateKeyed> { |
There was a problem hiding this comment.
The code removes the import for or-data-viewer but doesn't provide any migration path or deprecation notice in the changelog/documentation. Users of page-insights who depend on the dataViewer config may experience breaking changes without proper guidance.
| dataViewer?: any // deprecated DataViewerConfig | |
| } | |
| export function pageInsightsProvider(store: Store<AppStateKeyed>, config?: PageInsightsConfig): PageProvider<AppStateKeyed> { | |
| /** | |
| * @deprecated The dataViewer configuration has been removed from page-insights. | |
| * Please migrate any dataViewer usage to the dashboard builder configuration | |
| * supported by page-insights instead. | |
| */ | |
| dataViewer?: any; | |
| } | |
| export function pageInsightsProvider(store: Store<AppStateKeyed>, config?: PageInsightsConfig): PageProvider<AppStateKeyed> { | |
| if (config && (config as any).dataViewer) { | |
| const message = "The 'dataViewer' configuration for page-insights is deprecated and no longer has any effect. " + | |
| "Please migrate to the dashboard builder configuration used by page-insights."; | |
| // Log a deprecation warning for developers | |
| // eslint-disable-next-line no-console | |
| console.warn(message); | |
| // Optionally surface the deprecation to users in the UI | |
| try { | |
| showSnackbar({message}); | |
| } catch (_e) { | |
| // Ignore failures to show the snackbar to avoid breaking existing behavior | |
| } | |
| } |
| const util = require("@openremote/util"); | ||
| const { rspack } = require('@rspack/core'); | ||
| const packageJson = require('./package.json'); | ||
| const { RsdoctorRspackPlugin } = require('@rsdoctor/rspack-plugin'); |
There was a problem hiding this comment.
The import statement uses require() instead of ES6 import syntax for the RsdoctorRspackPlugin. This is inconsistent with the other imports at the top of the file which use ES6 module syntax. Consider using import { RsdoctorRspackPlugin } from '@rsdoctor/rspack-plugin'; for consistency.
| } | ||
|
|
||
| if (process.env.RSDOCTOR === "true") { | ||
| config.plugins.push(new RsdoctorRspackPlugin()) |
There was a problem hiding this comment.
Missing semicolon at the end of the statement. While JavaScript allows this, it's inconsistent with the style used elsewhere in the codebase and can lead to unexpected behavior with automatic semicolon insertion.
| config.plugins.push(new RsdoctorRspackPlugin()) | |
| config.plugins.push(new RsdoctorRspackPlugin()); |
…used code. (KPI widget)
… disabling correctly
… minor changes as well.
…dated logic from or-mwc-input to or-vaadin-input
|
…mbo-box items change detection. Realm selector in header now disables correctly.
|
|
Description
Work in progress.
Checklist