Code cleanup with ts options: noUnusedParameters, noUnusedLocals#599
Code cleanup with ts options: noUnusedParameters, noUnusedLocals#599uldisrudzitis merged 2 commits intomasterfrom
Conversation
WalkthroughThis PR removes numerous unused parameters and imports across components and rendering services, simplifies preview iframe reload logic, tightens TypeScript strictness with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI / Template
participant PreviewSvc as PreviewService
participant Iframe as Iframe (embedded)
rect #f0f8ff
Note over UI,PreviewSvc: Previous flow (before PR)
UI->>PreviewSvc: settingsChanged / store actions
PreviewSvc->>PreviewSvc: connectIframeReload -> subscribe to actions$
PreviewSvc->>Iframe: trigger reload/unload
Iframe-->>PreviewSvc: onbeforeunload -> disconnectIframeView
end
rect #f7fff0
Note over UI,PreviewSvc: New flow (after PR)
UI->>PreviewSvc: settingsChanged / store actions
PreviewSvc->>PreviewSvc: handle rerender via RerenderService only
PreviewSvc->>Iframe: update styles / rerender (no action subscription)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
editor/src/app/sites/sections/entries/galleries/gallery-render.service.ts (1)
250-256: Bug: video item src set to length, not URLitem.src uses item.poster.length (number) instead of the poster URL, breaking links and srcset concat.
Apply:
- if (item.type === 'video') { - item.videoLink = item.original; - item.src = item.poster.length ? item.poster.length : '#'; - } + if (item.type === 'video') { + item.videoLink = item.original; + item.src = item.poster && item.poster.length ? item.poster : '#'; + }Optionally only append no_cache when src is a URL:
- item.src += `?no_cache=${Date.now()}`; + if (item.src && item.src !== '#') { + item.src += `?no_cache=${Date.now()}`; + }editor/src/app/sites/sections/entries/galleries/gallery-row-render.service.ts (1)
32-35: Width calculation adds extra trailing spacingCurrent reduce adds SPACE_BETWEEN_ITEMS after the last item, inflating width.
Apply:
- return galleryItems.reduce((width: number, galleryItem) => { - return width + galleryItem.width + this.SPACE_BETWEEN_ITEMS; - }, 0); + return galleryItems.reduce( + (total: number, galleryItem, i) => + total + + galleryItem.width + + (i < galleryItems.length - 1 ? this.SPACE_BETWEEN_ITEMS : 0), + 0, + );editor/src/app/sites/sections/entries/galleries/gallery-pile-render.service.ts (1)
14-22: Return type mismatch and missing null‑safety in getGalleryStyles.Method returns null but is typed as string. Also, galleryItems may be undefined.
Apply:
- getGalleryStyles(galleryItems): string { - if (!galleryItems.length) { + getGalleryStyles(galleryItems): string | null { + if (!galleryItems?.length) { return null; } const item = galleryItems[0]; - const height = item.height ? item.height : item.width * 0.5625; // 16:9 ratio + const height = item?.height ? item.height : item?.width * 0.5625; // 16:9 ratio - return `width:${item.width}px;height:${height}px`; + return item?.width ? `width:${item.width}px;height:${height}px` : null; }
♻️ Duplicate comments (7)
editor/src/app/sites/sections/entries/galleries/gallery-slideshow-render.service.ts (2)
156-167: Stop passing nulls after signature shrinkAfter the change above, remove trailing nulls in the render call.
Apply:
- const viewData = this.getSlideshowViewData( - siteSlug, - entry, - siteSettings, - templateName, - siteTemplateSettings, - isLoopAvailable, - asRowGallery, - null, - null, - null, - ); + const viewData = this.getSlideshowViewData( + siteSlug, + entry, + siteSettings, + templateName, + siteTemplateSettings, + isLoopAvailable, + asRowGallery, + );
9-16: Class list type consistencyThis relies on super.getGalleryClassList returning an array; aligns with the base refactor I suggested to always return an array and join in view data. No changes here if base fix is applied.
editor/src/app/sites/sections/entries/galleries/gallery-row-render.service.ts (1)
133-142: Prune nulls after signature shrinkRemove the three nulls when calling getRowViewData if you accept the signature change.
Apply:
- const viewData = this.getRowViewData( - appState, - siteSlug, - entry, - siteSettings, - siteTemplateSettings, - null, - null, - null, - ); + const viewData = this.getRowViewData( + appState, + siteSlug, + entry, + siteSettings, + siteTemplateSettings, + );editor/src/app/sites/sections/entries/galleries/gallery-column-render.service.ts (2)
8-12: Class list return type consistencySame note as base/other renderers: keep base returning an array and join to string at view-data boundary. This override looks fine once base is consistent.
84-92: Stop passing nulls after signature shrinkAdjust render call accordingly.
Apply:
- const viewData = this.getColumnViewData( - siteSlug, - entry, - siteSettings, - siteTemplateSettings, - null, - null, - null, - ); + const viewData = this.getColumnViewData( + siteSlug, + entry, + siteSettings, + siteTemplateSettings, + );editor/src/app/sites/sections/entries/galleries/gallery-link-render.service.ts (2)
8-16: Class list return type consistencyMatches the pattern used elsewhere. OK once base emits array, and you join here.
83-109: Prune nulls after signature shrinkAlign render call to the simplified signature.
Apply:
- const viewData = this.getLinkViewData( - siteSlug, - entry, - siteSettings, - siteTemplateSettings, - null, - null, - null, - ); + const viewData = this.getLinkViewData( + siteSlug, + entry, + siteSettings, + siteTemplateSettings, + );
🧹 Nitpick comments (11)
editor/src/app/sites/sections/sections-menu-render.service.ts (1)
144-191: Consider refactoring the filter logic for clarity.The current
.filter()implementation applies the same boolean condition to all tags rather than filtering individual items. Since the logic determines whether to show the entire submenu (all tags or no tags), consider refactoring for better intent:const filteredTags = (() => { switch (templateName) { case 'messy': if (siteTemplateSettings.tagsMenu.hidden === 'yes') { return []; } if ( isResponsive && siteTemplateSettings.tagsMenu.alwaysOpen !== 'yes' && currentSection.name !== section.name ) { return []; } break; case 'white': if (currentSection.name !== section.name) { return []; } break; } // mashup template shows all submenus open // default template has separate submenu return sectionTags .sort((a, b) => a.order - b.order) .map((tag) => { return { name: tag['@attributes'].name, title: tag['@value'], attributes: toHtmlAttributes({ class: this.getSubmenuItemClassList( section, tag, currentSection, tagSlug, ), }), linkAttributes: toHtmlAttributes({ class: 'handle', href: this.getUrl(section, siteSlug, tag['@attributes'].name), }), }; }); })();This makes it explicit that the logic controls visibility of the entire submenu rather than filtering individual tags.
editor/src/app/shop/products/shop-products.component.ts (1)
72-101: Good fix: preserving non-matching products is critical.The explicit
elsebranch (lines 96-98) ensures that products whoseuniqiddoesn't match the current entry are preserved inleftOverProductsfor potential matching against subsequent entries. Without this branch, those products would be lost during the reduction.While this aligns with the PR's parameter-cleanup objective (removing the unused index parameter), it's more significant—it fixes a logic bug.
Optional: Consider refactoring for clarity.
The nested
reduceoperations are complex and may benefit from extraction into helper functions or a more readable data transformation pattern (e.g., usingfilterandmapwhere appropriate). This would improve maintainability without changing behavior.Example refactor approach:
- Extract the inner product-matching logic into a named function
- Consider using
filter/mapcombinations instead of nested reduces where applicable- Add type definitions to replace
any[](line 48, 71, etc.)editor/src/app/sites/media/site-media.component.ts (1)
82-82: Useentry.idfor tracking entries in the @for loop.Tracking by
entryuses object identity, which may cause unnecessary re-renders if entry objects are recreated during filtering or mapping operations. TheSectionEntryinterface has a stableid: stringproperty that should be used as a unique identifier instead.-@for (entry of selectedSection.entries; track entry) { +@for (entry of selectedSection.entries; track entry.id) {editor/src/app/sites/sections/entries/galleries/gallery-slideshow-render.service.ts (1)
64-75: Remove output-parameter pattern; compute locals insidegalleryItemsData/galleryItems/galleryType are derived inside; accepting them then overriding adds noise and invites nulls at call sites.
Apply:
- getSlideshowViewData( - siteSlug, - entry, - siteSettings, - templateName, - siteTemplateSettings, - isLoopAvailable, - asRowGallery, - galleryItemsData, - galleryItems, - galleryType, - ): { [key: string]: any } { - galleryItemsData = this.getGalleryItemsData(entry); - galleryItems = this.generateGalleryItems( + getSlideshowViewData( + siteSlug, + entry, + siteSettings, + templateName, + siteTemplateSettings, + isLoopAvailable, + asRowGallery, + ): { [key: string]: any } { + const galleryItemsData = this.getGalleryItemsData(entry); + const galleryItems = this.generateGalleryItems( siteSlug, galleryItemsData, entry, siteSettings, ); - galleryType = + const galleryType = entry.mediaCacheData && entry.mediaCacheData['@attributes'] && entry.mediaCacheData['@attributes'].type ? entry.mediaCacheData['@attributes'].type : siteTemplateSettings.entryLayout.defaultGalleryType;editor/src/app/sites/sections/entries/galleries/gallery-row-render.service.ts (2)
48-67: Loader height should follow last visible itemUsing the last item of the whole gallery can mismatch the visible loader block size.
Apply:
- const loaderWidth = this.getGalleryWidth( - galleryItems.slice(0, galleryItemsLimit), - ); - const lastItem = galleryItems.slice(-1)[0]; + const limit = Number.isFinite(galleryItemsLimit) + ? galleryItemsLimit + : galleryItems.length; + const visibleItems = galleryItems.slice(0, limit); + const loaderWidth = this.getGalleryWidth(visibleItems); + const lastItem = visibleItems[visibleItems.length - 1];
69-114: Remove output-parameter pattern; compute localsAs with slideshow, derive galleryItemsData/galleryItems/galleryType locally.
Apply:
- getRowViewData( - appState, - siteSlug, - entry, - siteSettings, - siteTemplateSettings, - galleryItemsData, - galleryItems, - galleryType, - ): { [key: string]: any } { - galleryItemsData = this.getGalleryItemsData(entry); - galleryItems = this.generateGalleryItems( + getRowViewData( + appState, + siteSlug, + entry, + siteSettings, + siteTemplateSettings, + ): { [key: string]: any } { + const galleryItemsData = this.getGalleryItemsData(entry); + const galleryItems = this.generateGalleryItems( siteSlug, galleryItemsData, entry, siteSettings, ); - galleryType = + const galleryType = entry.mediaCacheData && entry.mediaCacheData['@attributes'] && entry.mediaCacheData['@attributes'].type ? entry.mediaCacheData['@attributes'].type : siteTemplateSettings.entryLayout.defaultGalleryType;editor/src/app/sites/sections/entries/galleries/gallery-column-render.service.ts (1)
24-65: Remove output-parameter pattern; compute localsDerive galleryItemsData/galleryItems/galleryType inside; trims signature and avoids nulls at call sites.
Apply:
- getColumnViewData( - siteSlug, - entry, - siteSettings, - siteTemplateSettings, - galleryItemsData, - galleryItems, - galleryType, - ): { [key: string]: any } { - galleryItemsData = this.getGalleryItemsData(entry); - galleryItems = this.generateGalleryItems( + getColumnViewData( + siteSlug, + entry, + siteSettings, + siteTemplateSettings, + ): { [key: string]: any } { + const galleryItemsData = this.getGalleryItemsData(entry); + const galleryItems = this.generateGalleryItems( siteSlug, galleryItemsData, entry, siteSettings, ); - galleryType = + const galleryType = entry.mediaCacheData && entry.mediaCacheData['@attributes'] && entry.mediaCacheData['@attributes'].type ? entry.mediaCacheData['@attributes'].type : siteTemplateSettings.entryLayout.defaultGalleryType;editor/src/app/sites/sections/entries/galleries/gallery-link-render.service.ts (1)
28-81: Remove output-parameter pattern; compute localsSame cleanup as other renderers.
Apply:
- getLinkViewData( - siteSlug, - entry, - siteSettings, - siteTemplateSettings, - galleryItemsData, - galleryItems, - galleryType, - ): { [key: string]: any } { - galleryItemsData = this.getGalleryItemsData(entry); - galleryItems = this.generateGalleryItems( + getLinkViewData( + siteSlug, + entry, + siteSettings, + siteTemplateSettings, + ): { [key: string]: any } { + const galleryItemsData = this.getGalleryItemsData(entry); + const galleryItems = this.generateGalleryItems( siteSlug, galleryItemsData, entry, siteSettings, ); - galleryType = + const galleryType = entry.mediaCacheData && entry.mediaCacheData['@attributes'] && entry.mediaCacheData['@attributes'].type ? entry.mediaCacheData['@attributes'].type : siteTemplateSettings.entryLayout.defaultGalleryType;editor/src/app/sites/sections/entries/galleries/gallery-pile-render.service.ts (3)
67-82: Add explicit return type to render.Small clarity win and prevents accidental return type drift.
Apply:
- render(siteSlug, siteSettings, entry, siteTemplateSettings) { + render(siteSlug, siteSettings, entry, siteTemplateSettings): string {
84-92: Stop passing null placeholders; call the simplified getPileViewData.After removing unused parameters, this call should be simplified.
Apply:
- const viewData = this.getPileViewData( - siteSlug, - entry, - siteSettings, - siteTemplateSettings, - null, - null, - null, - ); + const viewData = this.getPileViewData( + siteSlug, + entry, + siteSettings, + siteTemplateSettings, + );
24-65: Remove unused parameters from getPileViewData; declare locally computed variables as const.The parameters
galleryItemsData,galleryItems, andgalleryTypeare reassigned on the first three lines of the method body, making them unused. The sole call site inrender()already passesnullfor these parameters. This refactoring eliminates violations of thenoUnusedParameterscompiler option.Apply the suggested diff to remove the three unused parameters from the method signature and declare the locally computed variables as
constinstead of reassigning parameters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
editor/src/app/inputs/file-input.component.ts(2 hunks)editor/src/app/popup/popup.component.ts(2 hunks)editor/src/app/preview/preview.component.ts(0 hunks)editor/src/app/preview/preview.service.ts(1 hunks)editor/src/app/preview/style.service.ts(1 hunks)editor/src/app/render/template-render.service.ts(0 hunks)editor/src/app/shared/loading/loading.component.ts(1 hunks)editor/src/app/shop/products/shop-products.component.ts(1 hunks)editor/src/app/shop/settings/shop-settings.component.ts(0 hunks)editor/src/app/sites/media/site-media.component.ts(2 hunks)editor/src/app/sites/sections/entries/galleries/gallery-column-render.service.ts(4 hunks)editor/src/app/sites/sections/entries/galleries/gallery-link-render.service.ts(4 hunks)editor/src/app/sites/sections/entries/galleries/gallery-pile-render.service.ts(4 hunks)editor/src/app/sites/sections/entries/galleries/gallery-render.service.ts(2 hunks)editor/src/app/sites/sections/entries/galleries/gallery-row-render.service.ts(4 hunks)editor/src/app/sites/sections/entries/galleries/gallery-slideshow-render.service.ts(4 hunks)editor/src/app/sites/sections/entries/section-entry-render.service.ts(0 hunks)editor/src/app/sites/sections/section-head-render.service.ts(0 hunks)editor/src/app/sites/sections/sections-menu-render.service.ts(1 hunks)editor/src/app/sites/sections/site-sections.component.ts(1 hunks)editor/src/app/sites/sites-state/sites.state.ts(1 hunks)editor/src/app/user/user.service.ts(0 hunks)editor/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (6)
- editor/src/app/user/user.service.ts
- editor/src/app/preview/preview.component.ts
- editor/src/app/render/template-render.service.ts
- editor/src/app/shop/settings/shop-settings.component.ts
- editor/src/app/sites/sections/entries/section-entry-render.service.ts
- editor/src/app/sites/sections/section-head-render.service.ts
🧰 Additional context used
🧬 Code graph analysis (1)
editor/src/app/popup/popup.component.ts (1)
editor/src/app/popup/popup.interface.ts (1)
PopupAction(13-17)
🔇 Additional comments (16)
editor/src/app/shared/loading/loading.component.ts (1)
1-1: LGTM!Removing the unused
OnInitimport is correct—the component doesn't implement the lifecycle hook and has no initialization logic. This aligns perfectly with the PR objective to enable stricter TypeScript compiler options.editor/src/app/sites/sections/sections-menu-render.service.ts (1)
145-145: Correct removal of unused parameter.The
tagparameter was never referenced in the filter predicate, as the logic only depends on template settings and section state. Removing it properly enables thenoUnusedParametersTypeScript option.editor/tsconfig.json (1)
4-5: LGTM! Stricter TypeScript checking enabled.Enabling
noUnusedLocalsandnoUnusedParametersimproves code quality by catching unused variables and parameters at compile time. This aligns well with the broader cleanup efforts across the codebase.editor/src/app/sites/sections/site-sections.component.ts (1)
197-212: LGTM! Unused parameter removed correctly.The
stateparameter was not referenced in the callback body. The current implementation correctly usesstore.selectSnapshotto access the required state instead.editor/src/app/inputs/file-input.component.ts (2)
57-57: LGTM! Template updated to match simplified method signature.
127-137: LGTM! Unused parameter removed correctly.The
fileInputparameter was never used in the method body. The method clearsthis.valuedirectly without needing a reference to the input element.editor/src/app/popup/popup.component.ts (2)
25-25: LGTM! Template updated to match simplified method signature.
131-137: LGTM! Unused parameter removed correctly.The
eventparameter was never used in the method body. The method only needs theactionto determine whether to execute a callback or close the popup.editor/src/app/sites/sites-state/sites.state.ts (1)
79-94: LGTM! Unused destructured parameter removed correctly.The
setStateparameter was extracted from theStateContextbut never used in the method body. Onlydispatchis needed for the initialization logic.editor/src/app/preview/style.service.ts (1)
242-259: LGTM! Unused variable removed correctly.The
fontStylevariable was extracted but never used. OnlyfontNameis needed for the CSS rules. Note that the fullgoogleFontSetting.value(including any style specifications like weights) is still passed toWebFont.loadon line 248, so font variants are loaded correctly.editor/src/app/preview/preview.service.ts (3)
3-5: LGTM! Imports streamlined after dependency removal.Also applies to: 14-14
33-38: LGTM! Constructor simplified after removing unused dependencies.The removal of
renderServiceandactions$parameters aligns with the elimination of the action-driven iframe reload mechanism. The service now has a more focused responsibility.
422-427: LGTM! Iframe reload logic simplified.The method now focuses solely on catching external links, with the complex action-driven reload mechanism removed. This simplification makes the code more maintainable.
editor/src/app/sites/media/site-media.component.ts (1)
73-73: Good simplification—tracking by unique property name.This change correctly tracks by the section's unique name property, which is clearer and more maintainable than the previous helper method approach.
editor/src/app/sites/sections/entries/galleries/gallery-link-render.service.ts (1)
66-79: The attribute name is correct as written.The XML source data in
sample-data/blog.[section-name].xmlconfirms the actual attribute islinkTarget(camelCase), notlink_target. While this is inconsistent with other attributes likelink_addressandrow_gallery_padding, the code ingallery-link-render.service.tscorrectly accesses it asentry.mediaCacheData['@attributes'].linkTarget.Likely an incorrect or invalid review comment.
editor/src/app/sites/sections/entries/galleries/gallery-render.service.ts (1)
235-243: Stabilize galleryClassList as a string in view dataBase getGalleryClassList currently returns an array while templates expect a string — make the base getter consistently typed as an array and always pass a joined string into the view data.
Apply (example) changes:
- getGalleryClassList(galleryItemsData, galleryType): string[] | string { + getGalleryClassList(galleryItemsData, galleryType): string[] { let classes = ['xGalleryContainer']; if (galleryItemsData.length) { classes.push('xGalleryHasImages'); classes.push(`xGalleryType-${galleryType}`); } - return classes; + return classes; }- galleryClassList: this.getGalleryClassList(galleryItemsData, galleryType), + galleryClassList: this.getGalleryClassList(galleryItemsData, galleryType).join(' '),Files/locations to change (from search):
- editor/src/app/sites/sections/entries/galleries/gallery-render.service.ts — getter and view-data assignment (~lines 235 and 299).
- editor/src/app/sites/sections/entries/galleries/gallery-row-render.service.ts — view data (~line 105).
- editor/src/app/sites/sections/entries/galleries/gallery-column-render.service.ts — view data (~line 57).
- editor/src/app/sites/sections/entries/galleries/gallery-link-render.service.ts — view data (~line 61).
- editor/src/app/sites/sections/entries/galleries/gallery-pile-render.service.ts — view data (~line 57).
- editor/src/app/sites/sections/entries/galleries/gallery-slideshow-render.service.ts — uses getSlideshowGalleryClassList (~line 100); ensure that getter either returns a string[] or its call site uses .join(' ').
Verification: run:
- rg -n "galleryClassList" editor/src | sed -n '1,200p'
- Confirm all assignments use .join(' ') (or that the called getter returns a string).
| getGalleryClassList(galleryItemsData, galleryType): string[] | string { | ||
| let classes = super.getGalleryClassList(galleryItemsData, galleryType); | ||
|
|
||
| return (classes as string[]).join(' '); | ||
| } |
There was a problem hiding this comment.
Fix unsafe cast and align return type in getGalleryClassList.
super.getGalleryClassList may return a string or string[]. Calling join on a string will throw at runtime. Also, this method always returns a string but is typed as string[] | string.
Apply:
- getGalleryClassList(galleryItemsData, galleryType): string[] | string {
- let classes = super.getGalleryClassList(galleryItemsData, galleryType);
-
- return (classes as string[]).join(' ');
- }
+ getGalleryClassList(galleryItemsData, galleryType): string {
+ const classes = super.getGalleryClassList(galleryItemsData, galleryType);
+ if (Array.isArray(classes)) {
+ return classes.join(' ');
+ }
+ return classes || '';
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getGalleryClassList(galleryItemsData, galleryType): string[] | string { | |
| let classes = super.getGalleryClassList(galleryItemsData, galleryType); | |
| return (classes as string[]).join(' '); | |
| } | |
| getGalleryClassList(galleryItemsData, galleryType): string { | |
| const classes = super.getGalleryClassList(galleryItemsData, galleryType); | |
| if (Array.isArray(classes)) { | |
| return classes.join(' '); | |
| } | |
| return classes || ''; | |
| } |
🤖 Prompt for AI Agents
In
editor/src/app/sites/sections/entries/galleries/gallery-pile-render.service.ts
around lines 8-12, the method unsafely casts the super result to string[] and
always calls join; change the method to return a string (update signature to
string) and handle both possible super return types safely by checking if
classes is an array then return classes.join(' ') else return classes as string;
remove the unsafe cast and ensure the function consistently returns a single
string.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
editor/src/app/sites/sections/entries/galleries/gallery-render.service.ts (1)
252-252: Consider simplifying the poster check.While functionally correct, you could simplify the condition for better readability:
- item.src = item.poster.length ? item.poster : '#'; + item.src = item.poster ? item.poster : '#';Since
posteris always a string (from line 83), a direct truthy check is more concise and equally effective—empty strings are falsy in JavaScript.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
editor/src/app/sites/sections/entries/galleries/gallery-render.service.ts(3 hunks)
🔇 Additional comments (2)
editor/src/app/sites/sections/entries/galleries/gallery-render.service.ts (2)
235-243: LGTM! Clean signature simplification.The removal of unused
entryandsiteSettingsparameters aligns well with the PR's objective. The method now has a focused signature that matches its actual usage, and the logic correctly builds the gallery class list based only on the data it needs.
290-308: AllgetGalleryClassListcall sites have been correctly updated to the new 2-parameter signature.Verification confirms the base class method definition (line 235) and all 10 call sites across 6 files (gallery-slideshow-render.service.ts, gallery-row-render.service.ts, gallery-link-render.service.ts, gallery-pile-render.service.ts, gallery-column-render.service.ts) use only the two required parameters:
galleryItemsDataandgalleryType. The refactoring is complete and consistent throughout the codebase.
1 participant
Summary by CodeRabbit
Refactor
Bug Fixes / Behavior
Chores