Conversation
uldisrudzitis
added
Enhancement
dependencies
WalkthroughAdds runtime Sentry initialization via APP_INITIALIZER and a new SentryConfigService, upgrades Angular/tooling and Node engines, swaps tooltip library, changes TypeScript moduleResolution to "bundler", extends a settings format union, removes Protractor E2E, and mass-refactors component templates from Angular structural directives to @if/@for/@switch-style syntax. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Angular App Bootstrap
participant Init as APP_INITIALIZER (sentryInitFactory)
participant Svc as SentryConfigService
participant HTTP as HttpClient
participant Sentry as @sentry/angular
App->>Init: invoke APP_INITIALIZER
Init->>Svc: getSentryDsn()
Svc->>HTTP: GET /_api/v1/sentry-dsn (text)
alt DSN returned
HTTP-->>Svc: DSN string
Svc-->>Init: DSN
Init->>Sentry: Sentry.init({ dsn })
Note right of Sentry: Sentry initialized
else no DSN / error
HTTP-->>Svc: error or empty
Svc-->>Init: ''
Note right of Init: skip Sentry.init()
end
Init-->>App: initialization completes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
editor/src/app/inputs/files-input.component.ts (1)
76-83: Remove MP4 exception from file-size check
Current condition only rejects oversized non-MP4 files, allowing MP4s of any size. Enforce the generic limit instead:- if (file.type !== 'video/mp4' && file.size > this.maxFileSize) { + if (file.size > this.maxFileSize) {editor/src/app/inputs/file-input.component.ts (2)
109-117: Guard against empty selection; avoid files[0] NPE.Change event can fire with no file. Accessing files[0].size will throw.
- validate(fileInput: HTMLInputElement) { - if (fileInput.files[0].size > this.maxFileSize) { + validate(fileInput: HTMLInputElement) { + const file = fileInput.files && fileInput.files.length ? fileInput.files[0] : null; + if (!file) { + return false; + } + if (file.size > this.maxFileSize) { this.error = `File exceeds the maximum file-size limit ${ this.maxFileSize / 1024 / 1024 } MB`; return false; } return true; }
89-89: disabled should be boolean, not string.String-typed disabled can coerce truthy unexpectedly (e.g., "false"). Bindings to [disabled] require boolean.
- @Input() disabled: string; + @Input() disabled: boolean = false;Ensure callers pass boolean values.
editor/src/app/sites/media/entry-gallery.component.ts (1)
23-59: Guard the loop source; track by a stable key.If mediaCacheData or file is undefined, @for will error. Also track by src for stability.
- <div class="entry-gallery-items"> - @for (file of entry.mediaCacheData.file; track file) { + <div class="entry-gallery-items"> + @if (entry?.mediaCacheData?.file?.length) { + @for (file of entry.mediaCacheData.file; track file['@attributes'].src) { <div class="entry-gallery-item" > @if (file['@attributes'].type === 'image') { <div class="media image"> - <img - src="{{ currentSite.mediaUrl }}/{{ entry.mediafolder }}/_smallthumb_{{ file['@attributes'].src }}" - /> + <img + [src]="currentSite.mediaUrl + '/' + entry.mediafolder + '/_smallthumb_' + file['@attributes'].src" + [attr.alt]="file['@attributes'].src" + /> </div> } @if (file['@attributes'].type === 'video') { <div class="media video"> <video [attr.poster]=" file['@attributes'].poster_frame ? currentSite.mediaUrl + '/' + entry.mediafolder + '/' + file['@attributes'].poster_frame : null " > - <source - src="{{ currentSite.mediaUrl }}/{{ entry.mediafolder }}/{{ file['@attributes'].src }}" - type="video/mp4" - /> + <source + [src]="currentSite.mediaUrl + '/' + entry.mediafolder + '/' + file['@attributes'].src" + type="video/mp4" + /> </video> </div> } </div> - } + } + }editor/src/app/sites/media/entry-gallery-image-editor.component.ts (1)
219-248: Guard against undefined imageCroppedEvent in updateSize()
this.imageCroppedEventcan be undefined before the first crop event, causing a runtime error when editing width/height immediately after ready.updateSize(e) { - let value = parseInt(e.value); + if (!this.imageCroppedEvent) { + return; + } + let value = parseInt(e.value); @@ - if (e.field === 'width') { + if (e.field === 'width') { + const ev = this.imageCroppedEvent; const x2 = - this.imageCroppedEvent.cropperPosition.x1 + - value * - (this.imageCroppedEvent.cropperPosition.x2 / - this.imageCroppedEvent.imagePosition.x2); + ev.cropperPosition.x1 + + value * (ev.cropperPosition.x2 / ev.imagePosition.x2); this.position = { - ...this.imageCroppedEvent.cropperPosition, + ...ev.cropperPosition, x2, }; } @@ - if (e.field === 'height') { + if (e.field === 'height') { + const ev = this.imageCroppedEvent; this.position = { - ...this.imageCroppedEvent.cropperPosition, + ...ev.cropperPosition, y2: - this.imageCroppedEvent.cropperPosition.y1 + - value * - (this.imageCroppedEvent.cropperPosition.y2 / - this.imageCroppedEvent.imagePosition.y2), + ev.cropperPosition.y1 + + value * (ev.cropperPosition.y2 / ev.imagePosition.y2), }; }editor/src/app/sites/site.component.ts (1)
32-48: Boolean guard + ARIA for action buttons
- Using a tri-state modificationDisabled is brittle. Make it a boolean and set explicitly.
- Add aria-labels to improve accessibility.
- @if (!modificationDisabled) { + @if (!modificationDisabled) { <button [attr.title]=" site['@attributes'].published > 0 ? 'Unpublish' : 'Publish' " + [attr.aria-label]=" + site['@attributes'].published > 0 ? 'Unpublish site' : 'Publish site' + " (click)=" updateField( '@attributes.published', site['@attributes'].published > 0 ? '0' : '1' ) " >Outside this range (component class), consider:
// Prefer explicit booleans modificationDisabled: boolean = false; ngOnInit() { this.hostname = location.hostname; this.modificationDisabled = this.site.name === ''; }Also applies to: 52-60
editor/src/app/sites/settings/site-settings.component.ts (1)
327-339: Guard against null user/appState in helpersuser$ and appState$ may be undefined initially; add defensive checks to prevent runtime errors.
isDisabled( settingGroup: SettingsGroupModel, setting: SettingModel, config: SettingConfigModel, user: UserStateModel ) { - return ( - this.settingUpdate[settingGroup.slug + ':' + setting.slug] || - (config.requires_feature && - user.features.indexOf(config.requires_feature) === -1) - ); + const updating = !!this.settingUpdate[settingGroup.slug + ':' + setting.slug]; + const lacksFeature = + !!config.requires_feature && + (!user || !user.features || user.features.indexOf(config.requires_feature) === -1); + return updating || lacksFeature; }disabledReason( config: SettingConfigModel, user: UserStateModel, appState: AppStateModel ) { - if ( - !config.requires_feature || - user.features.indexOf(config.requires_feature) > -1 - ) { - return; - } + if (!config?.requires_feature || !user?.features) return; + if (user.features.indexOf(config.requires_feature) > -1) return; - const requiredPlan = appState.plans.find( + const requiredPlan = appState?.plans?.find( (plan) => plan.features.indexOf(config.requires_feature) > -1 ); if (!requiredPlan) { return; }Also applies to: 340-360
🧹 Nitpick comments (44)
editor/src/app/inputs/files-input.component.ts (3)
72-73: Allow re‑selecting the same filesReset the input value after emit so a user can pick the same file(s) again.
- this.updateField(fileInput.files); + this.updateField(fileInput.files as FileList); + fileInput.value = '';
45-49: Use stable tracking to avoid duplicate collisionTracking by the error string collapses identical messages. Track by index.
- @for (error of errors; track error) { + @for (error of errors; track $index) {
7-7: Null‑safe error classAvoids potential undefined access before ngOnInit.
- [class.error]="errors.length > 0" + [class.error]="(errors?.length ?? 0) > 0"editor/tsconfig.json (1)
9-14: Align module with bundler resolution for Angular 20 toolchainWith "moduleResolution": "bundler", it's recommended to output modern ES modules. Consider updating "module" to ES2022 (or ESNext) to match target and avoid legacy transforms.
Apply:
- "module": "es2020", + "module": "ES2022",Optionally, use "ESNext" if your bundler (esbuild/Vite) prefers latest syntax. Verify local build and tests.
editor/src/app/user/user-account.component.ts (1)
13-19: Add ARIA for status messagesImprove accessibility so screen readers announce feedback.
Apply:
- <div class="error-message"> + <div class="error-message" role="alert" aria-live="assertive"> {{ error }} </div>- <div class="info-message">{{ message }}</div> + <div class="info-message" aria-live="polite">{{ message }}</div>Also applies to: 53-55
editor/src/app/shop/orders/shop-orders.component.ts (1)
11-12: Use stable track keys and provide an empty state
- Track by a stable id to reduce DOM churn.
- Provide @empty to handle no orders.
Apply:
- @for (order of orders$ | async; track order) { + @for (order of orders$ | async; track order.order_id) {- @for (item of order.orders; track item) { + @for (item of order.orders; track $index) {- } + } @empty { + <p class="setting">No orders found.</p> }If some orders lack order_id, choose another unique key (e.g., order.uid). Optionally hoist currency with @let to avoid multiple async subscriptions:
Example:
@let currency = (currency$ | async); ... <span>{{ currency }} {{ stringToCurrency(order.shipping) }}</span>Also applies to: 117-118, 128-129
editor/src/app/shop/products/shop-products.component.ts (1)
19-26: Track by stable keys to minimize re-rendersUse identifiers instead of object identity.
Apply:
- @for (group of productGroups$ | async; track group) { + @for (group of productGroups$ | async; track group.name) { ... - @for (product of group.products; track product) { + @for (product of group.products; track product.id) {Verify all groups have unique name and products have id. If not, fallback to track $index.
editor/src/app/sites/sites.component.ts (1)
19-25: Track sites by a unique fieldAvoid tracking by object identity.
Apply:
- @for (site of sitesList; track site) { + @for (site of sitesList; track site.name) {Confirm site.name is unique; otherwise, use another stable identifier.
editor/src/app/inputs/inline-text-input.component.ts (2)
93-110: Use KeyboardEvent.key; drop deprecated keyCode checks.keyCode is deprecated. Rely on
.keyonly for Escape/Enter.- if ( - $event instanceof KeyboardEvent && - ($event.key === 'Escape' || $event.keyCode === 27) - ) { + if ($event instanceof KeyboardEvent && $event.key === 'Escape') { ($event.target as HTMLInputElement).value = this.lastValue; ($event.target as HTMLInputElement).blur(); return; } @@ - if ( - $event instanceof KeyboardEvent && - !($event.key === 'Enter' || $event.keyCode === 13) - ) { + if ($event instanceof KeyboardEvent && $event.key !== 'Enter') { return; }
64-67: Keep lastValue in sync with external value changes.If parent updates [value] while not focused, Escape will revert to stale lastValue. Update lastValue in ngOnChanges when value changes and focus is false.
Additions outside the shown lines:
// imports import { Component, Input, Output, EventEmitter, OnInit, OnChanges, SimpleChanges } from '@angular/core'; // class signature export class InlineTextInputComponent implements OnInit, OnChanges { // ... ngOnChanges(changes: SimpleChanges) { if ('value' in changes && !this.focus) { this.lastValue = this.value; } } }editor/src/app/inputs/select-input.component.ts (1)
82-83: Type blurTimeoutId precisely.Use ReturnType to avoid any.
- blurTimeoutId: any; + blurTimeoutId: ReturnType<typeof setTimeout> | null = null;editor/src/app/inputs/file-input.component.ts (3)
15-15: Show the selected file name instead of “[object File]”.When value is a File, interpolation prints “[object File]”. Provide a display accessor.
- <div class="file-input">{{ value || 'Add file...' }}</div> + <div class="file-input">{{ displayValue || 'Add file...' }}</div>Add in the class (outside shown lines):
get displayValue(): string | null { if (!this.value) return null; return this.value instanceof File ? this.value.name : String(this.value); }
102-107: Allow re-selecting the same file; clear input after handling change.Browsers don’t fire change if the same file is reselected unless you reset the input value.
onChange(fileInput: HTMLInputElement) { if (!this.validate(fileInput) || this.disabled) { return; } - this.updateField(fileInput.files[0]); + this.updateField(fileInput.files![0]); + // Reset to allow selecting the same file again + fileInput.value = ''; }
119-129: Also clear the native input when removing a file.Keeps UI/state consistent and enables re-uploading the same file.
removeFile($event, fileInput: HTMLInputElement) { $event.stopPropagation(); $event.preventDefault(); if (this.disabled) { return; } this.value = ''; + if (fileInput) { + fileInput.value = ''; + } this.updateField(this.value); }editor/src/app/popup/popup.component.ts (2)
80-128: Unsubscribe on destroy to avoid leaks.Component subscribes to a service stream but never unsubscribes.
-import { Component, OnInit } from '@angular/core'; +import { Component, OnInit, OnDestroy } from '@angular/core'; -export class PopupComponent implements OnInit { +export class PopupComponent implements OnInit, OnDestroy { + private subscription: { unsubscribe(): void } | null = null; ngOnInit() { - this.service.subscribe({ + this.subscription = this.service.subscribe({ next: (popupState: PopupState) => { // ... }, error: (err) => { console.error(err); }, }); } + + ngOnDestroy() { + this.subscription?.unsubscribe(); + }If the service returns an RxJS Subscription, this pattern will prevent retained component instances.
25-35: Consider a stable track key for actions.Tracking by object identity can cause unnecessary re-renders. Use index or a semantic key if available (e.g., action.id or action.label).
- @for (action of actions; track action) { + @for (action of actions; track $index) {editor/src/app/sites/media/entry-gallery.component.ts (1)
73-75: Remove or use the unused mediaUrl field.mediaUrl is computed but not referenced in the template or class.
editor/src/app/sites/media/entry-gallery-image-editor.component.ts (1)
89-98: Make the Crop button actually disabled when action isn’t allowedCurrently only a CSS class toggles. Users can still click and trigger crop.
- <button + <button type="button" class="button" [class.disabled]="!canCrop" + [disabled]="!canCrop" (click)="cropImage()" > Crop </button>editor/src/app/sites/media/entry-gallery-editor.component.ts (1)
356-358: Avoid function in @for track; use a stable property expressionUsing
identifyGalleryItem($index, file)causes extra calls and binds. Track directly by a unique property.- @for ( - file of currentEntry.mediaCacheData.file; track identifyGalleryItem($index, - file)) { + @for ( + file of currentEntry.mediaCacheData.file; + track file['@attributes'].src + ) {editor/src/app/themes/themes.component.ts (1)
109-114: Harden window.open against tabnabbingAdd
noopener(and nullify opener for Safari).- window.open(url, '_blank'); + const win = window.open(url, '_blank', 'noopener'); + if (win) { + try { + win.opener = null; + } catch {} + }editor/src/app/sites/media/site-media.component.ts (2)
35-66: Use stable keys in @for track to reduce churn.Tracking by whole objects is fragile. Prefer stable primitives.
- Sections: track section.section.name
- Tags: track tag['@attributes'].name
Example:
- @for (section of sectionsList; track section) { + @for (section of sectionsList; track section.section.name) { ... - @for (tag of section.tags; track tag) { + @for (tag of section.tags; track tag['@attributes'].name) {
72-87: Avoid calling component methods in track; use direct expressions.Method calls in track run every CD pass. Use property expressions instead.
- @for ( selectedSection of selectedSections; track identifySection($index, selectedSection.section)) { + @for (selectedSection of selectedSections; track selectedSection.section.name) { ... - } @for (entry of selectedSection.entries; track identifyEntry($index, entry)) { + } @for (entry of selectedSection.entries; track entry) {You can then remove identifySection/identifyEntry or keep for other uses.
editor/src/app/sites/sections/section.component.ts (2)
91-101: Track params by a stable key, not the object.Reduces re-renders when array identity changes.
- @for (param of params; track param) { + @for (param of params; track (param.setting?.id ?? param.setting?.name)) {
34-48: Use consistent string values for 'published' to match the model.Model declares published?: '0' | '1'. Consider removing numeric 1 for consistency.
- [1, '1'].includes(section['@attributes'].published) + section['@attributes'].published === '1'And similarly in the [published] binding.
editor/src/app/shop/regional-costs/shop-regional-costs.component.ts (2)
22-37: Track regions by id rather than object.Prevents unnecessary DOM churn when arrays are recreated.
- @for (region of regionalCosts$ | async; track region) { + @for (region of regionalCosts$ | async; track region.id) {
52-82: Track costs by id.Same rationale as regions.
- @for (cost of region.costs; track cost) { + @for (cost of region.costs; track cost.id) {editor/package.json (1)
18-26: Align Angular package versions and TypeScript range
- Bump all "@angular/*" deps (except cdk) to "^20.3.4" to match build-angular/cli and avoid peer/version mismatches.
- Ensure devDependency "typescript" is >=5.9.0 <6.0.0 as required by Angular v20.3.
editor/src/app/shop/settings/shop-settings.component.ts (1)
17-35: Use stable identifiers in @for track clausesTrack by slug to avoid unnecessary re-renders when arrays are recreated.
- @for (settingGroup of settings$ | async; track settingGroup) { + @for (settingGroup of settings$ | async; track settingGroup.slug) { <div class="subgroup"> <div class="setting"> <h4>{{ settingGroup.config.title || settingGroup.slug }}</h4> </div> - @for (setting of settingGroup.settings; track setting) { + @for (setting of settingGroup.settings; track setting.setting.slug) { <berta-setting [setting]="setting.setting" [config]="setting.config" [disabled]=" settingUpdate[settingGroup.slug + ':' + setting.setting.slug] " [error]="settingError[settingGroup.slug + ':' + setting.setting.slug]" (update)="updateSetting(settingGroup.slug, $event)" ></berta-setting> } </div> }editor/src/app/sites/sections/site-sections.component.ts (1)
39-50: Track @for items by a stable key (section name)Tracking the object reference may cause churn when arrays are recreated. Use the section name.
- @for (sd of sectionsList; track sd) { + @for (sd of sectionsList; track sd.section.name) { <berta-section cdkDrag [section]="sd.section" [isExpanded]="sd.section.name === currentSection" [params]="sd.params" [templateSectionTypes]="sectionTypes$ | async" [translations]="translations$ | async" (inputFocus)="updateComponentFocus($event)" (update)="updateSection(sd, $event)" ></berta-section> }editor/src/app/shared/interfaces.ts (1)
45-50: Keep Response and Model format unions in syncYou added new literals to SettingConfigModel.format but SettingConfigResponse.format doesn’t include them. Align to reduce mapping casts and future type errors.
Proposed update in SettingConfigResponse:
format?: | 'text' | 'longtext' + | 'url' + | 'richtext' | 'select' | 'fontselect' | 'toggle' | 'icon' + | 'icon-readonly' | 'image' | 'color' + | 'route' + | 'action' | boolean;editor/src/app/sites/shared/setting-row.component.ts (1)
11-20: Use direct @for iteration and stable track key (avoid keyvalue churn).Iterate the array directly and track by a stable slug to reduce re-renders; also simplify the class check.
Apply:
- @for (inputField of inputFields | keyvalue; track inputField) { + @for (inputField of inputFields; track inputField.setting.slug) { <berta-setting - [class.bt-auto-width]=" - ['icon-readonly'].indexOf(inputField.value.config.format) > -1 - " - [setting]="inputField.value.setting" - [config]="inputField.value.config" + [class.bt-auto-width]="inputField.config.format === 'icon-readonly'" + [setting]="inputField.setting" + [config]="inputField.config" (update)="updateField($event)" ></berta-setting> }editor/src/app/header/preview-toggle.component.ts (1)
14-41: Collapse to a single @if/@else to avoid double evaluation.Current template duplicates conditions. Use @if/@else for clarity.
- @if (!isPreviewActive) { - <svg - xmlns="http://www.w3.org/2000/svg" - width="20" - height="12.6" - version="1.1" - viewBox="0 0 20 12.6" - > - <path - d="m18.5 6.3q-1.7-2.6-4.2-4 0.6 1.1 0.6 2.5 0 2.1-1.5 3.5-1.5 1.5-3.5 1.5-2.1 0-3.5-1.5-1.5-1.5-1.5-3.5 0-1.4 0.6-2.5-2.5 1.2-4.2 4 1.5 2.2 3.7 3.6 2.2 1.4 4.8 1.4 2.6 0 4.8-1.4 2.2-1.4 3.7-3.6zm-7.9-4.2q0-0.2-0.1-0.4t-0.4-0.1q-1.4 0-2.3 1-1 1-1 2.3 0 0.2 0.1 0.4 0.1 0.1 0.4 0.1t0.4-0.1q0.1-0.1 0.1-0.4 0-1 0.6-1.6 0.6-0.6 1.6-0.6 0.2 0 0.4-0.1 0.1-0.1 0.1-0.4zm9.4 4.2q0 0.4-0.2 0.7-1.5 2.6-4.2 4.1-2.6 1.5-5.6 1.5-2.8 0-5.6-1.5-2.6-1.5-4.2-4.1-0.2-0.4-0.2-0.7t0.2-0.7q1.6-2.5 4.2-4.1 2.6-1.5 5.6-1.5 2.8 0 5.6 1.5 2.6 1.5 4.2 4.1 0.2 0.4 0.2 0.7z" - stroke-width="0" - /> - </svg> - } - @if (isPreviewActive) { - <svg - xmlns="http://www.w3.org/2000/svg" - width="20" - height="18.3" - version="1.1" - viewBox="0 0 20 18.3" - > - <path - d="m4.6 15v1.7h-4.6v-1.7zm4.6-1.7q0.3 0 0.6 0.2 0.2 0.2 0.2 0.6v3.3q0 0.3-0.2 0.6-0.2 0.2-0.6 0.2h-3.3q-0.3 0-0.6-0.2-0.2-0.2-0.2-0.6v-3.3q0-0.3 0.2-0.6 0.2-0.2 0.6-0.2zm2.1-5v1.7h-11.2v-1.7zm-8.3-6.7v1.7h-2.9v-1.7zm17.1 13.3v1.7h-9.6v-1.7zm-12.5-15q0.3 0 0.6 0.2 0.2 0.2 0.2 0.6v3.3q0 0.3-0.2 0.6-0.2 0.2-0.6 0.2h-3.3q-0.3 0-0.6-0.2-0.2-0.2-0.2-0.6v-3.3q0-0.3 0.2-0.6 0.2-0.2 0.6-0.2zm8.3 6.7q0.3 0 0.6 0.2t0.2 0.6v3.3q0 0.3-0.2 0.6t-0.6 0.2h-3.3q-0.3 0-0.6-0.2-0.2-0.2-0.2-0.6v-3.3q0-0.3 0.2-0.6 0.2-0.2 0.6-0.2zm4.2 1.7v1.7h-2.9v-1.7zm0-6.7v1.7h-11.2v-1.7z" - stroke-width="0" - /> - </svg> - } + @if (isPreviewActive) { + <svg xmlns="http://www.w3.org/2000/svg" width="20" height="18.3" version="1.1" viewBox="0 0 20 18.3"> + <path d="m4.6 15v1.7h-4.6v-1.7zm4.6-1.7q0.3 0 0.6 0.2 0.2 0.2 0.2 0.6v3.3q0 0.3-0.2 0.6-0.2 0.2-0.6 0.2h-3.3q-0.3 0-0.6-0.2-0.2-0.2-0.2-0.6v-3.3q0-0.3 0.2-0.6 0.2-0.2 0.6-0.2zm2.1-5v1.7h-11.2v-1.7zm-8.3-6.7v1.7h-2.9v-1.7zm17.1 13.3v1.7h-9.6v-1.7zm-12.5-15q0.3 0 0.6 0.2 0.2 0.2 0.2 0.6v3.3q0 0.3-0.2 0.6-0.2 0.2-0.6 0.2h-3.3q-0.3 0-0.6-0.2-0.2-0.2-0.2-0.6v-3.3q0-0.3 0.2-0.6 0.2-0.2 0.6-0.2zm8.3 6.7q0.3 0 0.6 0.2t0.2 0.6v3.3q0 0.3-0.2 0.6t-0.6 0.2h-3.3q-0.3 0-0.6-0.2-0.2-0.2-0.2-0.6v-3.3q0-0.3 0.2-0.6 0.2-0.2 0.6-0.2zm4.2 1.7v1.7h-2.9v-1.7zm0-6.7v1.7h-11.2v-1.7z" stroke-width="0"/> + </svg> + } @else { + <svg xmlns="http://www.w3.org/2000/svg" width="20" height="12.6" version="1.1" viewBox="0 0 20 12.6"> + <path d="m18.5 6.3q-1.7-2.6-4.2-4 0.6 1.1 0.6 2.5 0 2.1-1.5 3.5-1.5 1.5-3.5 1.5-2.1 0-3.5-1.5-1.5-1.5-1.5-3.5 0-1.4 0.6-2.5-2.5 1.2-4.2 4 1.5 2.2 3.7 3.6 2.2 1.4 4.8 1.4 2.6 0 4.8-1.4 2.2-1.4 3.7-3.6zm-7.9-4.2q0-0.2-0.1-0.4t-0.4-0.1q-1.4 0-2.3 1-1 1-1 2.3 0 0.2 0.1 0.4 0.1 0.1 0.4 0.1t0.4-0.1q0.1-0.1 0.1-0.4 0-1 0.6-1.6 0.6-0.6 1.6-0.6 0.2 0 0.4-0.1 0.1-0.1 0.1-0.4zm9.4 4.2q0 0.4-0.2 0.7-1.5 2.6-4.2 4.1-2.6 1.5-5.6 1.5-2.8 0-5.6-1.5-2.6-1.5-4.2-4.1-0.2-0.4-0.2-0.7t0.2-0.7q1.6-2.5 4.2-4.1 2.6-1.5 5.6-1.5 2.8 0 5.6 1.5 2.6 1.5 4.2 4.1 0.2 0.4 0.2 0.7z" stroke-width="0"/> + </svg> + }Additionally, add an accessible label to the button:
<button class="bt-view-editor" type="button" (click)="togglePreview()" [attr.aria-label]="isPreviewActive ? 'Exit preview' : 'Enter preview'">editor/src/app/profile-dropdown/profile-dropdown.component.ts (1)
17-58: Cache the user with @let and use @else; add rel for external link.This avoids multiple async subscriptions and improves security for target="_blank".
- @if (!(user$ | async).profileUrl) { + @let user = (user$ | async); + @if (!user?.profileUrl) { <li> <a [routerLink]="['/account']" [routerLinkActive]="'nav-active'" queryParamsHandling="preserve" > Account <svg xmlns="http://www.w3.org/2000/svg" width="13.3" height="16" version="1.1" viewBox="0 0 13.3 16" > <path d="m13.3 13.2q0 1.1-0.7 1.9t-1.6 0.8h-8.9q-0.9 0-1.6-0.8t-0.7-1.9q0-0.9 0.1-1.7 0.1-0.8 0.3-1.6 0.2-0.8 0.6-1.4t1-0.9q0.6-0.4 1.4-0.4 1.4 1.3 3.3 1.3 1.9 0 3.3-1.3 0.8 0 1.4 0.4 0.6 0.4 1 0.9t0.6 1.4q0.2 0.8 0.3 1.6 0.1 0.8 0.1 1.7zm-2.7-9.2q0 1.7-1.2 2.8-1.2 1.2-2.8 1.2t-2.8-1.2q-1.2-1.2-1.2-2.8 0-1.7 1.2-2.8 1.2-1.2 2.8-1.2t2.8 1.2q1.2 1.2 1.2 2.8z" stroke-width="0" /> </svg> </a> </li> - } - @if ((user$ | async).profileUrl) { + } @else { <li> - <a href="{{ (user$ | async).profileUrl }}" target="_blank"> + <a [href]="user?.profileUrl" target="_blank" rel="noopener noreferrer"> Account <svg xmlns="http://www.w3.org/2000/svg" width="13.3" height="16" version="1.1" viewBox="0 0 13.3 16" > <path d="m13.3 13.2q0 1.1-0.7 1.9t-1.6 0.8h-8.9q-0.9 0-1.6-0.8t-0.7-1.9q0-0.9 0.1-1.7 0.1-0.8 0.3-1.6 0.2-0.8 0.6-1.4t1-0.9q0.6-0.4 1.4-0.4 1.4 1.3 3.3 1.3 1.9 0 3.3-1.3 0.8 0 1.4 0.4 0.6 0.4 1 0.9t0.6 1.4q0.2 0.8 0.3 1.6 0.1 0.8 0.1 1.7zm-2.7-9.2q0 1.7-1.2 2.8-1.2 1.2-2.8 1.2t-2.8-1.2q-1.2-1.2-1.2-2.8 0-1.7 1.2-2.8 1.2-1.2 2.8-1.2t2.8 1.2q1.2 1.2 1.2 2.8z" stroke-width="0" /> </svg> </a> </li> - } + }Optional: reuse the same
userfor the button label to avoid another async subscription:{{ user?.name }}.editor/src/app/sites/shared/setting.component.ts (1)
152-158: Confirm HTML trust boundary for description.
bypassSecurityTrustHtmldisables sanitization. Ensureconfig.descriptionis trusted or pre-sanitized server-side to avoid XSS.Consider centralizing description rendering behind a utility that sanitizes/whitelists allowed tags.
editor/src/app/sites/template-settings/site-template-settings.component.ts (1)
26-79: Track by stable slugs and improve header link semantics.
- Use slug-based tracking to minimize DOM churn.
- Wrap the clickable title in an anchor for native link semantics (keyboard + a11y), rather than
role="link"on h3.- @for (settingGroup of templateSettings$ | async; track settingGroup) { + @for (settingGroup of templateSettings$ | async; track settingGroup.slug) { <div class="setting-group" [class.is-expanded]="camelifySlug(currentGroup) === settingGroup.slug" > - <h3 - [routerLink]="[ - '/design', - camelifySlug(currentGroup) === settingGroup.slug - ? '' - : slugifyCamel(settingGroup.slug) - ]" - queryParamsHandling="preserve" - role="link" - class="hoverable" - > - {{ settingGroup.config.title || settingGroup.slug }} - <svg + <h3 class="hoverable"> + <a + [routerLink]="[ + '/design', + camelifySlug(currentGroup) === settingGroup.slug + ? '' + : slugifyCamel(settingGroup.slug) + ]" + queryParamsHandling="preserve" + > + {{ settingGroup.config.title || settingGroup.slug }} + </a> + <svg class="drop-icon" width="10" height="6" viewBox="0 0 10 6" fill="none" xmlns="http://www.w3.org/2000/svg" > <path d="M9 1L4.75736 5.24264L0.514719 1" stroke="#9b9b9b" stroke-linecap="round" stroke-linejoin="round" /> </svg> </h3> <div class="settings" [@isExpanded]="camelifySlug(currentGroup) === settingGroup.slug" > - @for (setting of settingGroup.settings; track setting) { + @for (setting of settingGroup.settings; track setting.setting.slug) { <berta-setting [templateSlug]="settingGroup.templateSlug" [setting]="setting.setting" [config]="setting.config" [disabled]=" settingUpdate[settingGroup.slug + ':' + setting.setting.slug] " [error]="settingError[settingGroup.slug + ':' + setting.setting.slug]" (update)="updateSetting(settingGroup.slug, $event)" (emitAction)="emitAction($event)" ></berta-setting> } </div> </div> }editor/src/app/shop/shop.component.ts (1)
14-18: Track by stable key to prevent unnecessary re-rendersTrack by slug instead of object identity to keep DOM stable across emissions.
- @for (shopSection of shopSections$ | async; track shopSection) { + @for (shopSection of shopSections$ | async; track shopSection.slug) {editor/src/app/login/login.component.ts (1)
25-26: Add alt text to logo for accessibility- <h3><img src="/engine/layout/berta.png" /></h3> + <h3><img src="/engine/layout/berta.png" alt="Berta CMS" /></h3>editor/src/app/sites/settings/site-settings.component.ts (1)
32-36: Use stable track keys for @for loopsTrack by unique slugs or indices to avoid churn on new object emissions.
- @for (settingGroup of settings$ | async; track settingGroup) { + @for (settingGroup of settings$ | async; track settingGroup.slug) { ... - @for (setting of settingGroup.settings; track setting) { + @for (setting of settingGroup.settings; track setting.setting.slug) { ... - @for (inputFields of setting.children; track inputFields; let index = $index) { + @for (inputFields of setting.children; let index = $index; track index) {Also applies to: 69-72, 97-113
editor/src/app/header/header.component.ts (1)
62-64: Add rel to external link opened in new tab- <a href="https://support.berta.me" target="_blank">Knowledge base</a> + <a href="https://support.berta.me" target="_blank" rel="noopener noreferrer">Knowledge base</a>editor/src/app/sites/sections/background-gallery-editor.component.ts (5)
35-56: Add keyboard/a11y semantics to the file settings toggleUse role, tabindex, aria-expanded and key handlers so the toggle is operable via keyboard and announced by AT.
Apply:
- <h3 - (click)="fileSettingsIsOpen = !fileSettingsIsOpen" - class="hoverable" - > + <h3 + (click)="fileSettingsIsOpen = !fileSettingsIsOpen" + (keydown.enter)="fileSettingsIsOpen = !fileSettingsIsOpen" + (keydown.space)="fileSettingsIsOpen = !fileSettingsIsOpen" + class="hoverable" + role="button" + tabindex="0" + [attr.aria-expanded]="fileSettingsIsOpen" + aria-controls="file-settings" + > @@ - <div class="settings" [@isExpanded]="fileSettingsIsOpen"> + <div class="settings" [@isExpanded]="fileSettingsIsOpen" id="file-settings">
73-95: Do the same a11y treatment for the gallery settings toggleMirror the accessibility improvements for the gallery settings section.
- <h3 - (click)="gallerySettingsIsOpen = !gallerySettingsIsOpen" - class="hoverable" - > + <h3 + (click)="gallerySettingsIsOpen = !gallerySettingsIsOpen" + (keydown.enter)="gallerySettingsIsOpen = !gallerySettingsIsOpen" + (keydown.space)="gallerySettingsIsOpen = !gallerySettingsIsOpen" + class="hoverable" + role="button" + tabindex="0" + [attr.aria-expanded]="gallerySettingsIsOpen" + aria-controls="gallery-settings" + > @@ - <div class="settings" [@isExpanded]="gallerySettingsIsOpen"> + <div class="settings" [@isExpanded]="gallerySettingsIsOpen" id="gallery-settings">
284-286: Avoid function call in @for track; use a stable expressionCalling a method per item increases CD cost. Track directly by a unique field.
- @for ( - file of currentSection.mediaCacheData?.file; track identifyGalleryItem($index, - file)) { + @for (file of currentSection.mediaCacheData?.file; track file['@attributes'].src) {Additionally, remove the now-unused identifyGalleryItem method:
// Remove if unused elsewhere // identifyGalleryItem(_, item: SiteSectionBackgroundFile) { return item['@attributes'].src; }
296-301: Prefer property binding for img src and add alt/lazy hintsUse [src] to avoid double interpolation, add alt, and opt-in to lazy decoding.
- <img - draggable="false" - src="{{ currentSite.mediaUrl }}/{{ currentSection.mediafolder }}/_smallthumb_{{ file['@attributes'].src }}" - /> + <img + draggable="false" + [src]="currentSite.mediaUrl + '/' + currentSection.mediafolder + '/_smallthumb_' + file['@attributes'].src" + [attr.alt]="file['@attributes'].src" + loading="lazy" + decoding="async" + />
275-281: Optional: collapse duplicate @if (currentSection) blocksYou can wrap aside and content in a single @if to reduce duplication.
Example:
@if (currentSection) { <aside>...</aside> <div class="content">...</div> }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
editor/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
editor/angular.json(1 hunks)editor/package.json(1 hunks)editor/src/app/header/header.component.ts(1 hunks)editor/src/app/header/preview-toggle.component.ts(1 hunks)editor/src/app/inputs/file-input.component.ts(1 hunks)editor/src/app/inputs/files-input.component.ts(1 hunks)editor/src/app/inputs/inline-text-input.component.ts(1 hunks)editor/src/app/inputs/select-input.component.ts(3 hunks)editor/src/app/inputs/text-input.component.ts(1 hunks)editor/src/app/inputs/url-input.component.ts(1 hunks)editor/src/app/login/login.component.ts(1 hunks)editor/src/app/popup/popup.component.ts(1 hunks)editor/src/app/profile-dropdown/profile-dropdown.component.ts(2 hunks)editor/src/app/shared/interfaces.ts(1 hunks)editor/src/app/shop/orders/shop-orders.component.ts(1 hunks)editor/src/app/shop/products/shop-products.component.ts(1 hunks)editor/src/app/shop/regional-costs/shop-regional-costs.component.ts(2 hunks)editor/src/app/shop/settings/shop-settings.component.ts(1 hunks)editor/src/app/shop/shop.component.ts(1 hunks)editor/src/app/sites/media/entry-gallery-editor.component.ts(13 hunks)editor/src/app/sites/media/entry-gallery-image-editor.component.ts(6 hunks)editor/src/app/sites/media/entry-gallery.component.ts(2 hunks)editor/src/app/sites/media/site-media.component.ts(5 hunks)editor/src/app/sites/sections/background-gallery-editor.component.ts(9 hunks)editor/src/app/sites/sections/section.component.ts(2 hunks)editor/src/app/sites/sections/site-sections.component.ts(1 hunks)editor/src/app/sites/settings/site-settings.component.ts(3 hunks)editor/src/app/sites/shared/help-tooltip.component.ts(2 hunks)editor/src/app/sites/shared/setting-row-add.component.ts(1 hunks)editor/src/app/sites/shared/setting-row.component.ts(1 hunks)editor/src/app/sites/shared/setting.component.ts(1 hunks)editor/src/app/sites/shared/sites-shared.module.ts(2 hunks)editor/src/app/sites/site.component.ts(3 hunks)editor/src/app/sites/sites.component.ts(1 hunks)editor/src/app/sites/template-settings/site-template-settings.component.ts(1 hunks)editor/src/app/themes/themes.component.ts(1 hunks)editor/src/app/user/user-account.component.ts(4 hunks)editor/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
editor/src/app/sites/media/site-media.component.ts (1)
editor/src/app/sites/sections/sections-state/site-sections-state.model.ts (1)
SiteSectionStateModel(1-37)
🔇 Additional comments (9)
editor/src/app/inputs/url-input.component.ts (1)
7-73: LGTM! Clean migration to Angular v17+ control flow syntax.The template has been successfully migrated from
*ngIfto@ifblocks while preserving all logic, bindings, and accessibility attributes. The conditional rendering for the icon (textInputService.showIcon | async) and clear button (allowBlank && value) remains functionally equivalent.editor/angular.json (1)
125-150: Workspace-level schematics defaults look goodSettings align with Angular CLI conventions and complement project-level overrides.
editor/package.json (2)
35-35: New tooltip dependency looks good; ensure module/directive is imported where used.Confirm consumers import either NgxTooltipDirectivesModule (NgModule apps) or the specific TooltipStrDirective in standalone components. Consider setting global defaults via forRoot for consistent styling.
Based on learnings
14-14: Angular 20.3 supports Node.js 24
Angular v20.3 (CLI and build-angular) officially supports Node.js 24, so the engines entry is correct.editor/src/app/sites/shared/help-tooltip.component.ts (1)
6-13: Tooltip directive inputs validated
textColor,backgroundColor,borderColorandshowDelayare validTooltipOptionsinputs in ngx-tooltip-directives v20.2.x; current usage is correct.editor/src/app/sites/shared/sites-shared.module.ts (1)
25-25: Approve tooltip module swap: migration is complete—no leftover@amin-karimi/ng2-tooltip-directiveimports or baretooltipattributes detected.editor/src/app/sites/shared/setting.component.ts (1)
12-150: Solid migration to @switch/@case; bindings preserved.Cases map cleanly to prior inputs; event wiring and props look consistent.
editor/src/app/sites/settings/site-settings.component.ts (1)
123-127: Confirm HTML source for getSettingDescriptionBypassing security assumes trusted source. If config can be user-controlled, sanitize upstream.
Do setting descriptions originate only from trusted, static config? If not, we should sanitize or restrict allowed tags.
editor/src/app/sites/sections/background-gallery-editor.component.ts (1)
28-75: Good migration to @if blocksTemplate conversion to Angular control flow reads cleanly. No functional regressions spotted in this section.
| [class.bt-disabled]="disabled" | ||
| > |
There was a problem hiding this comment.
Disable coercion + null-safe files check
disabled is used as a boolean but can be passed as a bare attribute (string), causing the control to be permanently disabled. Also guard against a null FileList.
Apply this diff for the null-safe check:
- if (this.disabled || !fileInput.files.length) {
+ if (this.disabled || !fileInput.files || fileInput.files.length === 0) {
return;
}And update the input to coerce booleans (outside these lines):
// add to imports
import { booleanAttribute } from '@angular/core';
// replace the input declaration
@Input({ transform: booleanAttribute }) disabled = false;Also applies to: 16-16, 68-68
🤖 Prompt for AI Agents
In editor/src/app/inputs/files-input.component.ts around lines 8-9 (also applies
to lines 16 and 68), the template and component treat "disabled" as a plain
boolean but it may be passed as a bare string attribute, and FileList checks are
not null-safe; import booleanAttribute from @angular/core and change the input
declaration to use transform: booleanAttribute (i.e. add the import and replace
the @Input with @Input({ transform: booleanAttribute }) disabled = false), and
update any file-list checks in the template and component to guard against
null/undefined (e.g., check files?.length or files && files.length) so the
control isn’t permanently disabled and null FileList won’t throw.
| ngSortgridItem | ||
| [ngSortGridItems]="currentSection.mediaCacheData.file" | ||
| (sorted)="reorder($event)" | ||
| (click)="setSelectedFile(file)" |
There was a problem hiding this comment.
🧩 Analysis chain
Double-check ng-sortgrid directive selector casing
Attribute is written as ngSortgridItem while the input is [ngSortGridItems]. Ensure the directive selector matches the library’s expected casing to avoid a no-op.
Run to verify usage consistency across the repo:
🏁 Script executed:
#!/bin/bash
# Show all occurrences of the directive attribute to compare casing
rg -n -C2 -P '\bngSortgridItem\b|\bngSortGridItem\b|\[ngSortGridItems\]'Length of output: 1532
Fix directive attribute casing for ngSortGridItem
Replace ngSortgridItem with ngSortGridItem in:
- editor/src/app/sites/media/entry-gallery-editor.component.ts (line 361)
- editor/src/app/sites/sections/background-gallery-editor.component.ts (line 290)
to match the library’s expected selector and ensure the directive applies.
🤖 Prompt for AI Agents
editor/src/app/sites/sections/background-gallery-editor.component.ts lines
290-293 and editor/src/app/sites/media/entry-gallery-editor.component.ts around
line 361: the directive attribute is incorrectly cased as ngSortgridItem; update
the attribute to ngSortGridItem in both templates so it matches the library
selector and the directive will be applied; search the repo for any other
occurrences of "ngSortgridItem" and replace them with "ngSortGridItem" to ensure
consistency.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
editor/src/app/sentry/sentry-config.service.ts (1)
11-20: Trim DSN to avoid stray whitespace issuesSafe micro-improvement; prevents falsey checks failing due to spaces/newlines.
- return dsn; + return dsn.trim();editor/src/app/sentry/sentry-init.factory.ts (1)
12-13: Consider wiring tracing and release/environmentOptional: add tracing and metadata for better observability.
+import { environment } from '../environments/environment'; @@ - Sentry.init({ dsn }); + Sentry.init({ + dsn, + // Enable browser tracing (adjust sample rate as needed) + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: environment.production ? 0.1 : 1.0, + environment: environment.production ? 'production' : 'development', + // Set release if available (e.g., injected at build time) + release: (window as any).SENTRY_RELEASE ?? undefined, + });editor/package.json (1)
30-32: Move @sentry/cli to devDependenciesIt’s a build-time tool; keeping it in dependencies bloats install surface unnecessarily.
"@sentry/angular": "^10.17.0", - "@sentry/cli": "^2.56.0", @@ "typescript": "~5.8.3" + , + "@sentry/cli": "^2.56.0"Also applies to: 45-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
editor/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
editor/.gitignore(1 hunks)editor/angular.json(2 hunks)editor/package.json(1 hunks)editor/src/app/app.module.ts(3 hunks)editor/src/app/sentry.error-handler.ts(0 hunks)editor/src/app/sentry/sentry-config.service.ts(1 hunks)editor/src/app/sentry/sentry-init.factory.ts(1 hunks)editor/src/app/sites/sections/site-sections.component.ts(3 hunks)
💤 Files with no reviewable changes (1)
- editor/src/app/sentry.error-handler.ts
✅ Files skipped from review due to trivial changes (1)
- editor/.gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- editor/angular.json
- editor/src/app/sites/sections/site-sections.component.ts
🧰 Additional context used
🧬 Code graph analysis (1)
editor/src/app/app.module.ts (1)
editor/src/app/sentry/sentry-init.factory.ts (1)
sentryInitFactory(4-14)
🔇 Additional comments (2)
editor/package.json (2)
15-16: Version compatibility confirmed: Node (^20.19.0‖^22.12.0‖^24.0.0), Angular core/CLI/build-angular (20.3.x), TypeScript (≥5.8 <6.0), and RxJS (7.8.x) are all mutually supported.
12-12: Confirm sourcemaps paths: Unable to locate an explicitoutputPathor existingdistdirectory—please verify that../engine/distmatches your build output and adjust the script accordingly.
Sentry integration update for backend
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
_api_app/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
_api_app/.env.example(1 hunks)_api_app/app/Http/Controllers/StateController.php(1 hunks)_api_app/composer.json(1 hunks)_api_app/config/sentry.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- _api_app/.env.example
🧰 Additional context used
📓 Path-based instructions (12)
_api_app/**/*.php
📄 CodeRabbit inference engine (_api_app/CLAUDE.md)
_api_app/**/*.php: Always use curly braces for control structures, even for single-line statements.
Use PHP 8 constructor property promotion in__construct()methods.
Do not allow empty__construct()methods with zero parameters.
Always use explicit return type declarations for methods and functions.
Use appropriate PHP type hints for method parameters.
Prefer PHPDoc blocks over inline comments. Avoid comments within code unless the logic is very complex.
Add useful array shape type definitions in PHPDoc blocks when working with arrays.
Enum keys should be TitleCase, for example:FavoritePerson,BestLake,Monthly.
Usephp artisan make:commands to create new Laravel files. Useartisan make:classfor generic PHP classes.
Use Laravel's built-in authentication and authorization features (gates, policies, Sanctum, etc.).
When generating links, prefer named routes and theroute()function.
_api_app/**/*.php: Always use curly braces for PHP control structures, even for single-line bodies
Use PHP 8 constructor property promotion in __construct() and avoid empty constructors
Always use explicit return type declarations for methods and functions
Use appropriate PHP type hints for method parameters
Prefer PHPDoc blocks over inline code comments; only add inline comments for very complex logic
Add useful PHPDoc array shape type definitions where appropriate
Enum case names should be TitleCase (e.g., FavoritePerson, BestLake, Monthly)
Prefer Eloquent models and relationships over raw database queries (avoid DB:: where possible; use Model::query())
Use Faker via $this->faker or fake() consistently with existing conventions in tests and factories
Files:
_api_app/config/sentry.php_api_app/app/Http/Controllers/StateController.php
_api_app/**/config/*.php
📄 CodeRabbit inference engine (_api_app/CLAUDE.md)
Use environment variables (
env()) only in configuration files. Elsewhere, useconfig()function.
Files:
_api_app/config/sentry.php
_api_app/config/**/*.php
📄 CodeRabbit inference engine (_api_app/.cursor/rules/laravel-boost.mdc)
Use env() only within configuration files
Access environment variables via env() within configuration files
Files:
_api_app/config/sentry.php
_api_app/{app,bootstrap,config,routes,database,tests}/**/*.php
📄 CodeRabbit inference engine (_api_app/AGENTS.md)
_api_app/{app,bootstrap,config,routes,database,tests}/**/*.php: Always use curly braces for control structures in PHP, even for single-line bodies
Use PHP 8 constructor property promotion in __construct() and do not include empty constructors
Always declare explicit return types and parameter type hints for functions and methods
Prefer PHPDoc blocks over inline comments; only comment complex logic and add useful array shape types when appropriate
Files:
_api_app/config/sentry.php_api_app/app/Http/Controllers/StateController.php
_api_app/**/app/Http/Controllers/*.php
📄 CodeRabbit inference engine (_api_app/CLAUDE.md)
Always create Form Request classes for validation rather than inline validation in controllers.
Files:
_api_app/app/Http/Controllers/StateController.php
_api_app/{app/Http/Controllers/**/*.php,app/Models/**/*.php}
📄 CodeRabbit inference engine (_api_app/.cursor/rules/laravel-boost.mdc)
Prevent N+1 queries by using eager loading when accessing relationships
Files:
_api_app/app/Http/Controllers/StateController.php
_api_app/{resources/views/**/*.blade.php,app/Http/Controllers/**/*.php}
📄 CodeRabbit inference engine (_api_app/.cursor/rules/laravel-boost.mdc)
Generate links using named routes and the route() helper
Files:
_api_app/app/Http/Controllers/StateController.php
_api_app/{app/**/*.php,bootstrap/**/*.php,routes/**/*.php,database/**/*.php}
📄 CodeRabbit inference engine (_api_app/.cursor/rules/laravel-boost.mdc)
Do not use env() outside configuration files; use config('...') instead
Files:
_api_app/app/Http/Controllers/StateController.php
_api_app/app/Http/Controllers/**/*.php
📄 CodeRabbit inference engine (_api_app/.cursor/rules/laravel-boost.mdc)
Do not perform inline validation in controllers; use dedicated Form Request classes
Use Form Request classes for validation instead of inline validation in controllers; include rules and custom messages
Files:
_api_app/app/Http/Controllers/StateController.php
_api_app/app/**/*.php
📄 CodeRabbit inference engine (_api_app/AGENTS.md)
Enum case names should be TitleCase (e.g., FavoritePerson, Monthly)
Files:
_api_app/app/Http/Controllers/StateController.php
_api_app/{app,bootstrap,routes,database,tests}/**/*.php
📄 CodeRabbit inference engine (_api_app/AGENTS.md)
_api_app/{app,bootstrap,routes,database,tests}/**/*.php: Avoid DB:: facade; prefer Model::query() and Eloquent ORM features
Never use env() outside config files; use config('app.name') etc. instead
Files:
_api_app/app/Http/Controllers/StateController.php
_api_app/{resources/views/**/*.blade.php,app/Http/Controllers/**/*.php,routes/**/*.php}
📄 CodeRabbit inference engine (_api_app/AGENTS.md)
Prefer named routes and route() for URL generation
Files:
_api_app/app/Http/Controllers/StateController.php
🔇 Additional comments (2)
_api_app/config/sentry.php (2)
26-26: No changes needed for Sentry documentation URL anchors. The underscore-separated fragments match the official docs.
35-37: Confirm sentry-laravel ≥ v4.15.0 for enable_logs support
Theenable_logsoption was introduced in sentry-php/sentry-laravel v4.15.0 and defaults tofalse. Ensure your project’s Sentry Laravel SDK dependency is updated to at least v4.15.0 to activate the logging integration.
Update Laravel dependencies
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
editor/package.json (1)
29-31: Move@sentry/clito devDependencies.
@sentry/cliis a build-time tool; keeping it independenciesforces it into production installs and containers, bloating the footprint. Shift it underdevDependenciesalongside the rest of the tooling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
_api_app/composer.lockis excluded by!**/*.lockeditor/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
editor/README.md(0 hunks)editor/angular.json(2 hunks)editor/e2e/protractor.conf.js(0 hunks)editor/e2e/src/app.e2e-spec.ts(0 hunks)editor/e2e/src/app.po.ts(0 hunks)editor/e2e/tsconfig.e2e.json(0 hunks)editor/package.json(1 hunks)
💤 Files with no reviewable changes (5)
- editor/e2e/tsconfig.e2e.json
- editor/README.md
- editor/e2e/src/app.po.ts
- editor/e2e/src/app.e2e-spec.ts
- editor/e2e/protractor.conf.js
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
editor/src/app/sites/media/entry-gallery.component.ts (1)
65-66: Remove unused properties.The
siteandmediaUrlproperties are computed but never referenced in the template, which usescurrentSite.mediaUrldirectly instead. These properties appear to be dead code.Apply this diff to remove the unused code:
- site = this.store.selectSnapshot(AppState.getSite); - mediaUrl = `/storage/${this.site ? `-sites/${this.site}/` : ''}media`; - constructor(private store: Store) {}
♻️ Duplicate comments (1)
editor/package.json (1)
11-11: sentry-cli upload will fail without an explicit releasePass a release to both inject and upload; otherwise sentry-cli exits with an error. Use the package version.
- "sentry:sourcemaps": "npm run build && sentry-cli sourcemaps inject --org hello-world-ik --project berta-frontend ../engine/dist && sentry-cli sourcemaps upload --org hello-world-ik --project berta-frontend ../engine/dist" + "sentry:sourcemaps": "npm run build && sentry-cli sourcemaps inject --org hello-world-ik --project berta-frontend --release $npm_package_version ../engine/dist && sentry-cli sourcemaps upload --org hello-world-ik --project berta-frontend --release $npm_package_version ../engine/dist"
🧹 Nitpick comments (3)
editor/package.json (1)
52-52: Remove leftover Protractor typings
@types/jasminewd2is for Protractor/WebDriverJS. With Protractor removed, this type package should be dropped.- "@types/jasminewd2": "~2.0.3",editor/src/app/sites/media/entry-gallery.component.ts (2)
20-49: Consider using@else ifor@switchfor mutually exclusive conditions.The type checking for image vs. video uses separate
@ifblocks (lines 20 and 28), but these conditions are mutually exclusive—a file can't be both an image and a video. Using@else ifor a@switchblock would make this logic clearer and slightly more efficient.Option 1: Use @else if
@if (file['@attributes'].type === 'image') { <div class="media image"> <img src="{{ currentSite.mediaUrl }}/{{ entry.mediafolder }}/_smallthumb_{{ file['@attributes'].src }}" /> </div> - } @if (file['@attributes'].type === 'video') { + } @else if (file['@attributes'].type === 'video') { <div class="media video"> <video [attr.poster]=" file['@attributes'].poster_frame ? currentSite.mediaUrl + '/' + entry.mediafolder + '/' + file['@attributes'].poster_frame : null " > <source src="{{ currentSite.mediaUrl }}/{{ entry.mediafolder }}/{{ file['@attributes'].src }}" type="video/mp4" /> </video> </div> }Option 2: Use @switch
- @if (file['@attributes'].type === 'image') { + @switch (file['@attributes'].type) { + @case ('image') { <div class="media image"> <img src="{{ currentSite.mediaUrl }}/{{ entry.mediafolder }}/_smallthumb_{{ file['@attributes'].src }}" /> </div> - } @if (file['@attributes'].type === 'video') { + } + @case ('video') { <div class="media video"> <video [attr.poster]=" file['@attributes'].poster_frame ? currentSite.mediaUrl + '/' + entry.mediafolder + '/' + file['@attributes'].poster_frame : null " > <source src="{{ currentSite.mediaUrl }}/{{ entry.mediafolder }}/{{ file['@attributes'].src }}" type="video/mp4" /> </video> </div> } + }
18-18: Consider a more specific track expression.The
@forloop tracks byfileobject reference, which works if file objects are immutable. However, if files have a unique identifier (e.g.,file['@attributes'].srcor an ID), tracking by that property would be more robust and improve change detection performance.For example:
- @for (file of entry.mediaCacheData.file; track file) { + @for (file of entry.mediaCacheData.file; track file['@attributes'].src) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
editor/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
editor/package.json(1 hunks)editor/src/app/sites/media/entry-gallery.component.ts(2 hunks)
🔇 Additional comments (3)
editor/package.json (2)
66-66: No changes needed: TypeScript ~5.8.3 satisfies Angular v20’s minimum TS 5.8 requirement.
14-14: No changes needed for Node 24 Angular v20 officially supports Node.js ^24.0.0 (alongside ^20.19.0 and ^22.12.0), so the engines entry is correct.editor/src/app/sites/media/entry-gallery.component.ts (1)
23-66: Remove unnecessary null check forcurrentSite.The parent
site-media.component.tsalready guards with@if (currentSite$ | async) { … <entry-gallery [currentSite]="currentSite$ | async"> … }, socurrentSiteinEntryGalleryComponentis never undefined at runtime.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
editor/src/app/sites/media/entry-gallery.component.ts (1)
26-47: Use@else ifinstead of standalone@iffor video block.Line 26 uses a standalone
@ifcondition instead of@else if, which breaks the mutually exclusive logic between image and video rendering. Iffile['@attributes'].typeis neither'image'nor'video', neither block will render. While this might be the intended fallback behavior, the pattern suggests these should be mutually exclusive branches.Apply this diff to ensure mutual exclusivity:
} - @if (file['@attributes'].type === 'video') { + @else if (file['@attributes'].type === 'video') { <div class="media video">editor/package.json (1)
54-65: Resolve the TypeScript 5.8 vs TSLint/Codelyzer peer conflict
tslint@6.1.0andcodelyzer@6.0.2both cap their TypeScript peer dependency at<5.0.0. Upgradingtypescriptto~5.8.3makesnpm installfail with anERESOLVE(or forces--legacy-peer-deps), so the project can’t install or build in its default workflow. Please drop these obsolete packages (or migrate to ESLint) before landing the TypeScript bump.Minimal fix:
- "codelyzer": "^6.0.2", ... - "tslint": "~6.1.0", - "typescript": "~5.8.3" + "typescript": "~5.8.3"(Or replace with the modern ESLint tooling if linting is still required.)
🧹 Nitpick comments (1)
editor/src/app/sites/media/entry-gallery.component.ts (1)
16-16: Consider tracking by unique identifier instead of object reference.The
track fileexpression tracks by object reference, which can cause unnecessary re-renders if the file objects are recreated with new references. Consider tracking by a unique identifier for better performance.Apply this diff to track by the file source:
- @for (file of entry.mediaCacheData.file; track file) { + @for (file of entry.mediaCacheData.file; track file['@attributes'].src) { <div class="entry-gallery-item">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
editor/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
editor/package.json(1 hunks)editor/src/app/sites/media/entry-gallery.component.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
editor/src/app/sites/media/entry-gallery.component.ts (1)
editor/src/app/sites/media/site-media.component.ts (1)
Component(17-228)
🔇 Additional comments (2)
editor/src/app/sites/media/entry-gallery.component.ts (2)
1-4: LGTM! Store dependency successfully removed.The imports correctly reflect the migration away from NGXS store, keeping only the essential Angular core imports and model types.
59-62: LGTM! Clean component class aligned with store removal.The component class correctly implements the migration away from store injection, relying solely on
@Inputproperties to receivecurrentSiteandentrydata.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
editor/src/app/sites/media/entry-gallery.component.ts (1)
19-48: Use@else iffor mutually exclusive conditions.The two separate
@ifblocks at lines 19 and 27 evaluate both conditions even when the first matches. Sincetypeshould be either 'image' or 'video' (mutually exclusive), chain them with@else iffor better performance and clarity.Apply this diff:
@if (file['@attributes'].type === 'image') { <div class="media image"> <img src="{{ currentSite.mediaUrl }}/{{ entry.mediafolder }}/_smallthumb_{{ file['@attributes'].src }}" /> </div> - } @if (file['@attributes'].type === 'video') { + } @else if (file['@attributes'].type === 'video') { <div class="media video"> <video [attr.poster]=" file['@attributes'].poster_frame ? currentSite.mediaUrl + '/' + entry.mediafolder + '/' + file['@attributes'].poster_frame : null " > <source src="{{ currentSite.mediaUrl }}/{{ entry.mediafolder }}/{{ file['@attributes'].src }}" type="video/mp4" /> </video> </div> }editor/package.json (3)
14-14: Node engines: confirm Node 24 support and intended Node 18 drop.Angular 20 toolchain may not be fully validated on Node 24 yet. Also, excluding Node 18 is a policy decision—verify it’s intentional.
If you prefer safer bounds, consider narrowing or including 18 LTS:
- "node": "^20.19.0 || ^22.12.0 || ^24.0.0" + "node": "^20.19.0 || ^22.0.0" +# or (if you still want to support 18 LTS) +# "node": "^18.19.0 || ^20.19.0 || ^22.0.0"
62-62: ts-node is too old for TS 5.8/Node 20+.v9.1.1 predates TS 5.x and Node 20 APIs; it will likely fail if used.
- "ts-node": "~9.1.1" + "ts-node": "^10.9.2"If you don’t use ts-node anywhere, remove it to reduce risk.
51-61: Modernize test stack for CLI v20.
- jasmine-core/~@types/jasmine 3.x are quite old; upgrade to Jasmine 5.x.
- Replace karma-coverage-istanbul-reporter with karma-coverage (the Istanbul reporter is deprecated).
- "@types/jasmine": "~3.8.0", - "jasmine-core": "~3.8.0", - "karma-coverage-istanbul-reporter": "~3.0.2", + "@types/jasmine": "^5.1.0", + "jasmine-core": "^5.2.0", + "karma-coverage": "^2.2.0",Adjust karma.conf.js accordingly (reporters: ['progress','coverage'], etc.).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
editor/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
editor/package.json(1 hunks)editor/src/app/pipes/pipe.ts(1 hunks)editor/src/app/sites/media/entry-gallery.component.ts(2 hunks)editor/src/tslint.json(0 hunks)editor/tslint.json(0 hunks)
💤 Files with no reviewable changes (2)
- editor/src/tslint.json
- editor/tslint.json
✅ Files skipped from review due to trivial changes (1)
- editor/src/app/pipes/pipe.ts
🧰 Additional context used
🧬 Code graph analysis (1)
editor/src/app/sites/media/entry-gallery.component.ts (1)
editor/src/app/sites/media/site-media.component.ts (1)
Component(17-228)
🔇 Additional comments (4)
editor/src/app/sites/media/entry-gallery.component.ts (1)
60-62: LGTM: Clean migration to input-based architecture.The removal of store dependency in favor of direct
@Inputproperties simplifies the component and improves testability. The inputs correctly match the parent component's data flow.editor/package.json (3)
18-26: Angular v20 dependency alignment looks good.Versions are consistent across core packages and ecosystem libs for this upgrade.
Also applies to: 28-29, 32-36, 40-40
43-47: No action needed—TS ~5.8.3 falls within Angular 20.3.3’s supported range (>=5.8 <6.0).
11-11: Verify production sourcemaps, outputPath, and release parity.
- In editor/angular.json under build → configurations → production, enable
"sourceMap": true(or the detailed object) to emit .map files.- Confirm
outputPath.baseremains"../engine/dist"(matches your Sentry CLI path).- Ensure the runtime Sentry.init release string exactly matches
$npm_package_versionand that this variable expands on Windows.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
editor/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
editor/package.json(1 hunks)
| "dev": "ng build --watch --configuration development", | ||
| "test": "ng test", | ||
| "e2e": "ng e2e" | ||
| "sentry:sourcemaps": "npm run build && sentry-cli sourcemaps inject --org hello-world-ik --project berta-frontend --release $npm_package_version ../engine/dist && sentry-cli sourcemaps upload --org hello-world-ik --project berta-frontend --release $npm_package_version ../engine/dist" |
There was a problem hiding this comment.
Make the Sentry script cross-platform
The sentry:sourcemaps command relies on $npm_package_version, which npm only expands under POSIX shells; on Windows it stays literal and the command fails. Please swap in a shell-agnostic lookup (e.g. --release "$(node -p \"require('./package.json').version\")" or wrap the script with cross-env) so the workflow succeeds on every developer machine.
🤖 Prompt for AI Agents
In editor/package.json around line 11, the sentry:sourcemaps script uses the
POSIX-only $npm_package_version which fails on Windows; update the script to
compute the release value in a cross-platform way — replace $npm_package_version
with a shell-agnostic lookup such as --release "$(node -p
\"require('./package.json').version\")" (or alternatively wrap the script with
cross-env/cross-env-shell to supply npm_package_version) so the sentry-cli
commands receive the correct release value on all platforms.
There was a problem hiding this comment.
Sorry @coderabbitai no windows support in this project :)
There was a problem hiding this comment.
@uldisrudzitis, understood! Thanks for clarifying—I'll keep that in mind for future reviews.
✏️ Learnings added
Learnt from: uldisrudzitis
PR: berta-cms/berta#576
File: editor/package.json:11-11
Timestamp: 2025-10-08T08:09:22.727Z
Learning: The Berta project does not support Windows as a platform, so cross-platform compatibility with Windows is not required.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
@coderabbitai do I need the internet explorer support? 😀
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for browserslist configuration
echo "=== Checking for .browserslistrc ==="
fd -t f "^\.browserslistrc$" .
echo -e "\n=== Checking package.json for browserslist config ==="
cat editor/package.json | jq -r '.browserslist // "No browserslist config in package.json"'
echo -e "\n=== Checking angular.json for browser target ==="
cat editor/angular.json | jq -r '.projects.berta.architect.build.options.tsConfig // "N/A"' 2>/dev/null || echo "Could not find tsConfig in angular.json"
echo -e "\n=== Searching for IE references in config files ==="
rg -i "internet.?explorer|\\bIE\\b|\\bIE[0-9]+" --type=json -g '!node_modules' -g '!package-lock.json' editor/ || echo "No IE references found"
Length of output: 486
🏁 Script executed:
#!/bin/bash
# Check for hidden .browserslistrc file
echo "=== Checking for hidden .browserslistrc ==="
fd -H -t f "^\.browserslistrc$" editor/ || echo "No .browserslistrc file found"
echo -e "\n=== Angular v20 default browser support info ==="
echo "Angular v20 supports: last 2 Chrome/Edge/Firefox/Safari versions + Chrome/Safari iOS"
echo "Internet Explorer has been unsupported since Angular v13 (2021)"
Length of output: 269
🏁 Script executed:
#!/bin/bash
# Check the contents of .browserslistrc
echo "=== Contents of editor/src/.browserslistrc ==="
cat editor/src/.browserslistrc
Length of output: 420
@uldisrudzitis, nope! 😄 Internet Explorer has been officially retired since June 2022, and Angular dropped IE support way back in v13 (2021). Angular v20 absolutely doesn't support it.
However, I did notice your .browserslistrc file still has this line:
IE 9-11
You should remove that line since it's no longer relevant and Angular v20 can't target IE anyway. The file is at editor/src/.browserslistrc.
Here's what you can keep for modern browser support:
> 0.5%
last 2 versions
Firefox ESR
not dead
This will target current browsers that Angular v20 actually supports (Chrome, Edge, Firefox, Safari). 🚀
1 participant
Summary by CodeRabbit
New Features
Refactor
Chores