Feat/angular-roadmap-svg-zoom #429
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces a new roadmap feature within the blog library. It adds complete configuration files for ESLint, Jest, and TypeScript along with project metadata. New Angular components, pipes, and style files enable rendering an interactive roadmap using SVG elements. Additionally, the changes update routing in the blog shell to include the roadmap feature and incorporate a new dependency for SVG manipulation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BlogShell
participant RoadmapShell as Roadmap Shell
participant FeatureRoadmap as FeatureRoadmap Component
participant UI as UI Components
User->>BlogShell: Navigate to /roadmap
BlogShell->>RoadmapShell: Load roadmap route
RoadmapShell->>FeatureRoadmap: Embed roadmap feature
FeatureRoadmap->>UI: Render nodes and layers
UI-->>FeatureRoadmap: Return rendered SVG nodes
RoadmapShell->>User: Display complete roadmap
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
DamianBrzezinskiHoA
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (18)
libs/blog/roadmap/feature-roadmap/src/lib/slice.pipes.ts (2)
3-11: Consider adding standalone: true to the pipe decoratorIf you're using Angular 14+ and want to make this pipe more reusable, you can add
standalone: trueto the pipe decorator. This allows importing the pipe directly into components without needing to include it in a module.@Pipe({ name: 'leftSlice', + standalone: true, })
13-21: Consider adding standalone: true to the pipe decoratorIf you're using Angular 14+ and want to make this pipe more reusable, you can add
standalone: trueto the pipe decorator. This allows importing the pipe directly into components without needing to include it in a module.@Pipe({ name: 'rightSlice', + standalone: true, })libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-secondary-node.component.ts (1)
9-11: Consider extracting hardcoded colors to design tokensThe inline style block contains hardcoded color values. For better maintainability and theming support, consider extracting these colors to a shared design token system or SCSS variables.
- style=" - --primary-color: #B3004A; --secondary-color: #66002B; --gradient-color: #481CAB; --on-hover-border-1: #923CFF; --on-hover-border-2: #FF006A" + [style]="roadmapNodeColors"You could then define these colors in the component class or better yet, in a shared service:
roadmapNodeColors = { '--primary-color': '#B3004A', '--secondary-color': '#66002B', '--gradient-color': '#481CAB', '--on-hover-border-1': '#923CFF', '--on-hover-border-2': '#FF006A' };libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-angular-love-node.component.ts (2)
5-21: Consider consolidating node components to reduce duplicationThis component has significant template and style duplication with UiRoadmapSecondaryNodeComponent and UiRoadmapPrimaryNodeComponent. The main differences are styling properties like margin size, text size, and background gradient.
Consider creating a base component that accepts configuration for these styling differences:
@Component({ selector: 'al-ui-roadmap-node', template: ` <div class="roadmap-hover-border-gradient relative w-fit text-nowrap rounded-lg bg-[#FDF5FD] text-[#FDF5FD]" [style]="styleVars" > <div class="relative z-10 rounded-lg px-6 py-4" [ngClass]="config.margin" [ngStyle]="backgroundStyle" > <div [ngClass]="config.textSize">{{ node().title }}</div> </div> </div> `, styleUrls: ['./roadmap-hover-border-gradient.scss'], }) export class UiRoadmapNodeComponent { readonly node = input.required<RoadmapNode>(); @Input() config: NodeConfig = defaultConfig; get styleVars() { return { '--primary-color': '#B3004A', '--secondary-color': '#66002B', '--gradient-color': '#481CAB', '--on-hover-border-1': '#923CFF', '--on-hover-border-2': '#FF006A' }; } get backgroundStyle() { return this.config.useGradient ? { 'background': 'linear-gradient(to right, var(--secondary-color), var(--gradient-color))' } : { 'background': 'var(--' + this.config.backgroundColor + ')' }; } } // Then create simple wrapper components for the specific node types
14-14: Note the duplicate space in class definitionThere are two consecutive spaces between
rounded-lgandbg-gradient-to-rin the class attribute. While this doesn't affect functionality, it's a minor styling inconsistency.- class="relative z-10 m-[4px] rounded-lg bg-gradient-to-r from-[--secondary-color] to-[--gradient-color] px-6 py-4" + class="relative z-10 m-[4px] rounded-lg bg-gradient-to-r from-[--secondary-color] to-[--gradient-color] px-6 py-4"libs/blog/shell/feature-shell-web/src/lib/blog-shell.routes.ts (1)
24-30: Consider adding SEO data for the roadmap routeThe roadmap route is missing SEO data that's present on most other routes in the file. This could affect search engine visibility for this feature.
{ path: 'roadmap', loadComponent: async () => await import('./roadmap-shell.component').then( (m) => m.RoadmapShellComponent, ), + data: { + seo: { title: 'seo.roadmap' }, + }, },libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-primary-node.component.ts (1)
8-18: Consider extracting hard-coded colors and styles.The template includes hard-coded CSS variables and styling. For better maintainability and theme support, consider extracting these color values to a shared constants file or theme variables.
- <div - class="roadmap-hover-border-gradient relative w-fit text-nowrap rounded-lg bg-[#FDF5FD] text-[#FDF5FD]" - style=" - --primary-color: #B3004A; --secondary-color: #66002B; --gradient-color: #481CAB; --on-hover-border-1: #923CFF; --on-hover-border-2: #FF006A" - > + <div + class="roadmap-hover-border-gradient relative w-fit text-nowrap rounded-lg bg-[#FDF5FD] text-[#FDF5FD]" + [ngClass]="themeClass" + >Consider creating a shared theme service or constants file for these values across all roadmap node components.
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-cluster.component.ts (3)
5-34: Consider extracting the template to a separate HTML fileThe inline template is becoming quite complex. For better maintainability and code organization, consider moving it to a separate HTML file.
28-32: Avoid hard-coded color values in the componentThe hard-coded CSS color values in the host binding should ideally be moved to a centralized theme configuration to maintain consistency across components and enable easier theming changes.
5-5: Consider adding OnPush change detectionFor better performance, consider adding
changeDetection: ChangeDetectionStrategy.OnPushto the component decorator since you're using signals which work well with OnPush change detection.-@Component({ +@Component({ + changeDetection: ChangeDetectionStrategy.OnPush, selector: 'al-ui-roadmap-cluster',libs/blog/roadmap/feature-roadmap/src/lib/ui/roadmap-hover-border-gradient.scss (3)
9-13: Consider optimizing the pseudo-element dimensionsThe fixed 2000px size for the pseudo-element is very large and might be excessive. Consider using a percentage-based or calculated size that scales with the element rather than a fixed pixel value.
- width: 2000px; - height: 2000px; - top: calc(50% - 1000px); - left: calc(50% - 1000px); + width: 300%; + height: 300%; + top: -100%; + left: -100%;
13-13: Use CSS variables for consistent colorsThe hardcoded color values (#ff006a, #923cff) should be replaced with CSS variables to maintain consistency with the component's use of variables for theming.
- background: conic-gradient(#ff006a, #923cff, #ff006a); + background: conic-gradient(var(--on-hover-border-2), var(--on-hover-border-1), var(--on-hover-border-2));
20-22: Optimize animation performanceFor better performance, consider adding the
will-change: transformproperty to hint the browser about the upcoming animation. Also, consider using thetransformproperty exclusively for animations as it's often hardware-accelerated.&:not(.cluster-node):hover::before { opacity: 1; animation: rotate 1s linear infinite; + will-change: transform; }libs/blog/roadmap/feature-roadmap/src/lib/secondary-arrow.pipe.ts (1)
11-31: Consider clarifying arc logic and adding tests.This arrow path generation is clear, but negative or large
horizontalShiftvalues may cause unexpected arcs. Consider adding tests to verify correct arrow shape edges for different input ranges.libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.html (1)
52-105: Validate absolute positioning offset logic.These inline positions (e.g.
style="position: absolute") and top/left coordinates assume certain container dimensions. If sizes change at runtime, arrow lines might become misaligned. Regularly test on different screen sizes or dynamic roadmap data.libs/blog/shell/feature-shell-web/src/lib/roadmap-shell.component.ts (2)
41-60: Unused adBannerVisible value.Since
adBannerVisiblealways returnsfalse, the layout offset logic in the effect might never apply. Consider either removing this unused signal or completing its implementation.
80-88: Temporary solution note.A
TODOindicates that banner layout logic is a temporary workaround. You might consider creating a dedicated service or stable approach for controlling viewport offset to avoid potential tech debt.libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.ts (1)
56-99: Consider externalizing roadmap data.Storing
nodesDtodata inline is convenient for a prototype. However, pulling this data from a config file or service can improve maintainability and clarity once this roadmap grows more complex.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
libs/blog/roadmap/feature-roadmap/.eslintrc.json(1 hunks)libs/blog/roadmap/feature-roadmap/README.md(1 hunks)libs/blog/roadmap/feature-roadmap/jest.config.ts(1 hunks)libs/blog/roadmap/feature-roadmap/project.json(1 hunks)libs/blog/roadmap/feature-roadmap/src/index.ts(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.html(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.scss(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.spec.ts(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.ts(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/secondary-arrow.pipe.ts(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/slice.pipes.ts(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/ui/roadmap-hover-border-gradient.scss(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-angular-love-node.component.ts(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-cluster.component.ts(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-primary-node.component.ts(1 hunks)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-secondary-node.component.ts(1 hunks)libs/blog/roadmap/feature-roadmap/src/test-setup.ts(1 hunks)libs/blog/roadmap/feature-roadmap/tsconfig.json(1 hunks)libs/blog/roadmap/feature-roadmap/tsconfig.lib.json(1 hunks)libs/blog/roadmap/feature-roadmap/tsconfig.spec.json(1 hunks)libs/blog/shell/feature-shell-web/src/lib/blog-shell.routes.ts(1 hunks)libs/blog/shell/feature-shell-web/src/lib/roadmap-shell.component.ts(1 hunks)libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts(1 hunks)package.json(1 hunks)tsconfig.base.json(1 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-secondary-node.component.ts (3)
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-primary-node.component.ts (1)
Component(5-24)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-angular-love-node.component.ts (1)
Component(5-24)libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.ts (2)
Component(40-211)RoadmapNode(24-28)
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-primary-node.component.ts (4)
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-cluster.component.ts (1)
Component(5-37)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-angular-love-node.component.ts (1)
Component(5-24)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-secondary-node.component.ts (1)
Component(5-24)libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.ts (2)
Component(40-211)RoadmapNode(24-28)
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-angular-love-node.component.ts (3)
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-primary-node.component.ts (1)
Component(5-24)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-secondary-node.component.ts (1)
Component(5-24)libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.ts (2)
Component(40-211)RoadmapNode(24-28)
libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.ts (4)
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-cluster.component.ts (1)
Component(5-37)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-primary-node.component.ts (1)
Component(5-24)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-angular-love-node.component.ts (1)
Component(5-24)libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-secondary-node.component.ts (1)
Component(5-24)
🪛 Biome (1.9.4)
libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.ts
[error] 103-103: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 158-158: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 177-177: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (29)
libs/blog/roadmap/feature-roadmap/src/index.ts (1)
1-1: Export Re-Export Verified.
This export statement correctly re-exports all public members from thefeature-roadmap.componentmodule, streamlining imports for downstream modules.package.json (1)
66-66: SVG-Pan-Zoom Dependency Added.
The addition of the"svg-pan-zoom": "^3.6.2"dependency introduces the necessary SVG manipulation features for the roadmap component. Please ensure that you periodically verify the security and compatibility status of this dependency.libs/blog/roadmap/feature-roadmap/README.md (1)
1-8: New README Documentation for Feature-Roadmap.
The README clearly outlines the purpose of the library and provides concise instructions for running unit tests withnx test feature-roadmap. Expanding it in the future with usage examples or further documentation may benefit users.libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts (1)
57-59: Host Styling Added for Layout Consistency.
The new host property assigning the class'flex flex-col min-h-screen'ensures that the component maintains a full-height flex layout—which should contribute positively to the overall UI consistency.libs/blog/roadmap/feature-roadmap/src/test-setup.ts (1)
1-7: Test Environment Setup Configured Correctly.
The test setup utilizingsetupZoneTestEnvwith botherrorOnUnknownElementsanderrorOnUnknownPropertiesset to true is a robust configuration for catching template errors early during testing.libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.scss (1)
1-12: Animation implementation looks goodThe fadeIn animation is well-implemented with a smooth 1-second ease-in-out transition from opacity 0 to 1. This provides a nice visual effect when the roadmap container is rendered.
tsconfig.base.json (1)
191-193: Path mapping correctly configuredThe path mapping for the new feature-roadmap module follows the established pattern in the project, enabling proper module resolution through the alias.
libs/blog/roadmap/feature-roadmap/project.json (1)
1-20: Project configuration looks goodThe configuration follows the standard NX workspace pattern for Angular libraries, with appropriate tags and correctly configured test and lint targets.
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-secondary-node.component.ts (2)
5-21: Well-structured component using appropriate styling techniquesThe component's template uses a combination of Tailwind CSS classes and CSS variables for styling, which provides flexibility. The hover gradient effect is applied consistently with other roadmap node components.
22-24: Input property correctly implemented with required modifierThe component properly uses Angular's input signal with the required modifier to ensure the node data is provided when using this component.
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-angular-love-node.component.ts (1)
22-24: Input property correctly implemented with required modifierThe component properly uses Angular's input signal with the required modifier, ensuring the node data is provided.
libs/blog/shell/feature-shell-web/src/lib/blog-shell.routes.ts (2)
24-30: Route placement places roadmap outside the main shellNote that the roadmap route is configured to exist outside of the RootShellComponent, unlike most other routes. This means it won't inherit the layout and UI elements from the root shell. Ensure this is intentional, as it will create a different user experience compared to other sections of the application.
26-29: Properly implemented lazy loading for the roadmap componentThe route correctly uses async/await with the import() syntax for lazy loading, which will help with initial load performance by only loading the roadmap feature when it's needed.
libs/blog/roadmap/feature-roadmap/tsconfig.lib.json (1)
1-17: Standard TypeScript library configurationThe TypeScript configuration follows standard practices for an Angular library within a monorepo structure. It correctly:
- Extends the base configuration
- Sets up declaration files and source maps
- Excludes test files from the library build
- Includes all TypeScript files in the source directory
libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.spec.ts (1)
1-22: Basic test structure looks good but could be enhanced.This is a minimal test file that verifies the component can be created, which is a good starting point. Consider expanding the test suite to cover more functionality as the component evolves.
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-primary-node.component.ts (1)
22-24: Input declaration looks good.The use of
input.required<RoadmapNode>()is a modern approach for declaring required inputs in Angular.libs/blog/roadmap/feature-roadmap/.eslintrc.json (1)
1-36: ESLint configuration follows Angular best practices.The ESLint configuration properly enforces consistent naming conventions with the 'al' prefix. Component selectors use kebab-case and directive selectors use camelCase, which aligns with Angular style guidelines.
libs/blog/roadmap/feature-roadmap/jest.config.ts (1)
1-21: Jest configuration looks good.The Jest configuration properly sets up everything needed for Angular component testing, including the necessary transforms and serializers.
libs/blog/roadmap/feature-roadmap/tsconfig.spec.json (1)
1-16: Configuration looks appropriate for Jest testingThe TypeScript configuration for tests is correctly set up with CommonJS module system, ES2016 target, and proper type definitions for Jest and Node. The file patterns for tests also follow Angular conventions.
libs/blog/roadmap/feature-roadmap/src/lib/ui/ui-roadmap-cluster.component.ts (2)
14-14: Optimize the trackBy function for better performanceThe current trackBy is using
clusterNode.idwhich is good, but ensure this id is stable and unique for optimal rendering performance.
36-36: Good use of the required input APIThe component correctly uses Angular's newer signal-based input API with required validation to ensure the cluster data is provided.
libs/blog/roadmap/feature-roadmap/src/lib/ui/roadmap-hover-border-gradient.scss (1)
16-16: Be cautious with z-index valuesThe z-index of 1 might cause layering issues with other components. Consider implementing a consistent z-index strategy across the application to prevent unexpected stacking context issues.
libs/blog/roadmap/feature-roadmap/tsconfig.json (1)
1-28: Well-configured TypeScript settings with strict type checkingThe TypeScript configuration is appropriately set up with strict type checking enabled, which will help catch potential errors early in development. The Angular compiler options correctly enforce strict template type checking as well.
libs/blog/roadmap/feature-roadmap/src/lib/secondary-arrow.pipe.ts (2)
1-3: Good use of Angular pipe boilerplate.The import declarations and pipe metadata are straightforward and adhere to Angular convention.
7-9: Parameter default assignment is succinct.Using
withArc = trueas a default parameter keeps the function flexible without cluttering the method signature.libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.html (2)
1-6: Adopt consistent SVG namespace usage.While
xmlns:svg="http://www.w3.org/1999/html"works, it looks somewhat unconventional for an SVG container. Verify if it’s the intended namespace or if you should swap to the standard"http://www.w3.org/2000/svg".
17-49: Efficient@foriteration with track.Using
@for (layer of roadmapLayers(); track layer.parentNode.id)is good for performance. Ensure each uniquelayer.parentNode.idremains stable if data changes.libs/blog/shell/feature-shell-web/src/lib/roadmap-shell.component.ts (1)
18-30: Simple and clean template structure.The template integrates the header, roadmap, and footer well. The approach is readable and straightforward.
libs/blog/roadmap/feature-roadmap/src/lib/feature-roadmap.component.ts (1)
101-210: Roadmap layering logic looks robust.The layered approach is coherent and methodically handles cluster nodes vs. primary nodes. Great job in ensuring cluster child ordering with
previousNodeId.🧰 Tools
🪛 Biome (1.9.4)
[error] 103-103: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 158-158: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 177-177: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
| import { Pipe, PipeTransform } from '@angular/core'; | ||
|
|
||
| @Pipe({ | ||
| name: 'leftSlice', | ||
| }) | ||
| export class LeftSlicePipe implements PipeTransform { | ||
| transform<T>(value: T[]): T[] { | ||
| const halfLength = Math.ceil(value.length / 2); | ||
| return value.slice(0, halfLength); | ||
| } | ||
| } | ||
|
|
||
| @Pipe({ | ||
| name: 'rightSlice', | ||
| }) | ||
| export class RightSlicePipe implements PipeTransform { | ||
| transform<T>(value: T[]): T[] { | ||
| const halfLength = Math.ceil(value.length / 2); | ||
| return value.slice(halfLength); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adjusting the slicing logic for odd-length arrays
The pipes are well-typed with generics, but there's a potential issue with how they handle odd-length arrays. When an array has an odd number of elements:
LeftSlicePipewill return the larger half (due toMath.ceil)RightSlicePipewill return the smaller half
This could cause unexpected behavior if these pipes are used together to split an array.
Consider one of these solutions:
@Pipe({
name: 'rightSlice',
})
export class RightSlicePipe implements PipeTransform {
transform<T>(value: T[]): T[] {
- const halfLength = Math.ceil(value.length / 2);
+ const halfLength = Math.floor(value.length / 2);
return value.slice(halfLength);
}
}Or for completely equal division (some overlap for odd lengths):
@Pipe({
name: 'leftSlice',
})
export class LeftSlicePipe implements PipeTransform {
transform<T>(value: T[]): T[] {
- const halfLength = Math.ceil(value.length / 2);
+ const halfLength = Math.floor(value.length / 2);
return value.slice(0, halfLength);
}
}📝 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.
| import { Pipe, PipeTransform } from '@angular/core'; | |
| @Pipe({ | |
| name: 'leftSlice', | |
| }) | |
| export class LeftSlicePipe implements PipeTransform { | |
| transform<T>(value: T[]): T[] { | |
| const halfLength = Math.ceil(value.length / 2); | |
| return value.slice(0, halfLength); | |
| } | |
| } | |
| @Pipe({ | |
| name: 'rightSlice', | |
| }) | |
| export class RightSlicePipe implements PipeTransform { | |
| transform<T>(value: T[]): T[] { | |
| const halfLength = Math.ceil(value.length / 2); | |
| return value.slice(halfLength); | |
| } | |
| } | |
| import { Pipe, PipeTransform } from '@angular/core'; | |
| @Pipe({ | |
| name: 'leftSlice', | |
| }) | |
| export class LeftSlicePipe implements PipeTransform { | |
| transform<T>(value: T[]): T[] { | |
| const halfLength = Math.ceil(value.length / 2); | |
| return value.slice(0, halfLength); | |
| } | |
| } | |
| @Pipe({ | |
| name: 'rightSlice', | |
| }) | |
| export class RightSlicePipe implements PipeTransform { | |
| transform<T>(value: T[]): T[] { | |
| const halfLength = Math.floor(value.length / 2); | |
| return value.slice(halfLength); | |
| } | |
| } |
| Object.entries(clusterMap).forEach(([clusterNodeId, childrenNodeIds]) => { | ||
| const previousClusterNodeIdToNodeIdMap = childrenNodeIds.reduce( | ||
| (acc, primaryNodeId) => ({ | ||
| ...acc, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid repeated spreading in clusterMap.
Again, consider directly assigning properties to the accumulator object rather than spreading to prevent repeated copying.
🧰 Tools
🪛 Biome (1.9.4)
[error] 158-158: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
| // setup layers | ||
| const previousLayerNodeIdToNodeIdMap = Object.keys(layerMap).reduce( | ||
| (acc, primaryNodeId) => ({ | ||
| ...acc, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Likewise, prefer direct assignment in reduce.
Repeated usage of ...acc in .reduce() can degrade performance in large data sets. In-place updates yield more efficient time complexity.
🧰 Tools
🪛 Biome (1.9.4)
[error] 177-177: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
DamianBrzezinskiHoA
force-pushed
the
feat/angular-roadmap-svg-zoom
branch
from
fe73f1d to
4bc6287
Compare
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
DamianBrzezinskiHoA
force-pushed
the
feat/angular-roadmap-svg-zoom
branch
from
4bc6287 to
a86f645
Compare
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
DO NOT CHECK
It is a feature that is mainly connected with angular-roadmap feature
Summary by CodeRabbit