diff --git a/docs/guidelines/code-style.md b/docs/guidelines/code-style.md index f2751121b..dc6a70456 100644 --- a/docs/guidelines/code-style.md +++ b/docs/guidelines/code-style.md @@ -5,6 +5,186 @@ - **Readability First**: Write code that is easy to read and understand. - **Consistency**: Follow the same patterns and conventions throughout the codebase. - **Clean Code**: Avoid unnecessary complexity and keep functions small and focused. +- **SOLID Principles**: Follow SOLID design principles to create more maintainable, flexible, and scalable code. + +## SOLID Design Principles + +SOLID is an acronym for five design principles that help make software designs more understandable, flexible, and maintainable: + +- **Single Responsibility Principle (SRP)**: A class should have only one reason to change, meaning it should have only one job or responsibility. + + ```typescript + // Good - Each class has a single responsibility + class UserAuthentication { + authenticate(username: string, password: string): boolean { + // Authentication logic + } + } + + class UserRepository { + findById(id: string): User { + // Database access logic + } + } + + // Bad - Class has multiple responsibilities + class UserManager { + authenticate(username: string, password: string): boolean { + // Authentication logic + } + + findById(id: string): User { + // Database access logic + } + + sendEmail(user: User, message: string): void { + // Email sending logic + } + } + ``` + +- **Open/Closed Principle (OCP)**: Software entities should be open for extension but closed for modification. + + ```typescript + // Good - Open for extension + interface PaymentProcessor { + processPayment(amount: number): void; + } + + class CreditCardProcessor implements PaymentProcessor { + processPayment(amount: number): void { + // Credit card processing logic + } + } + + class PayPalProcessor implements PaymentProcessor { + processPayment(amount: number): void { + // PayPal processing logic + } + } + + // New payment methods can be added without modifying existing code + ``` + +- **Liskov Substitution Principle (LSP)**: Objects of a superclass should be replaceable with objects of subclasses without affecting the correctness of the program. + + ```typescript + // Good - Derived classes can substitute base class + class Rectangle { + constructor( + protected width: number, + protected height: number, + ) {} + + setWidth(width: number): void { + this.width = width; + } + + setHeight(height: number): void { + this.height = height; + } + + getArea(): number { + return this.width * this.height; + } + } + + class Square extends Rectangle { + constructor(size: number) { + super(size, size); + } + + // Preserve behavior when overriding methods + setWidth(width: number): void { + super.setWidth(width); + super.setHeight(width); + } + + setHeight(height: number): void { + super.setWidth(height); + super.setHeight(height); + } + } + ``` + +- **Interface Segregation Principle (ISP)**: Clients should not be forced to depend on interfaces they do not use. + + ```typescript + // Good - Segregated interfaces + interface Printable { + print(): void; + } + + interface Scannable { + scan(): void; + } + + class AllInOnePrinter implements Printable, Scannable { + print(): void { + // Printing logic + } + + scan(): void { + // Scanning logic + } + } + + class BasicPrinter implements Printable { + print(): void { + // Printing logic + } + } + + // Bad - Fat interface + interface OfficeMachine { + print(): void; + scan(): void; + fax(): void; + staple(): void; + } + + // Classes must implement methods they don't need + ``` + +- **Dependency Inversion Principle (DIP)**: High-level modules should not depend on low-level modules. Both should depend on abstractions. + + ```typescript + // Good - Depending on abstractions + interface Logger { + log(message: string): void; + } + + class ConsoleLogger implements Logger { + log(message: string): void { + console.log(message); + } + } + + class FileLogger implements Logger { + log(message: string): void { + // File logging logic + } + } + + class UserService { + constructor(private logger: Logger) {} + + createUser(user: User): void { + // Create user logic + this.logger.log(`User created: ${user.name}`); + } + } + + // The UserService depends on the abstraction (Logger interface) + // not on concrete implementations + ``` + +Following these principles improves code quality by: + +- Reducing coupling between components +- Making the system more modular and easier to maintain +- Facilitating testing and extension +- Promoting code reuse ## Extended Guidelines for Angular and TypeScript @@ -13,16 +193,24 @@ This section extends the core code style principles with Angular-specific and ad ### Angular Enhancements - **Change Detection**: Use the OnPush strategy by default for better performance. -- **Lifecycle Hooks**: Explicitly implement Angular lifecycle interfaces. +- **Lifecycle Hooks**: Explicitly implement Angular lifecycle interfaces (OnInit, OnDestroy, etc.). - **Template Management**: Keep templates concise and use the async pipe to handle observables. - **Component Structure**: Follow best practices for component modularization to enhance readability and testability. +- **Naming Conventions**: Follow Angular's official naming conventions for selectors, files, and component classes. +- **File Organization**: Structure files according to features and follow the recommended folder structure. +- **Control Flow**: Use modern control flow syntax (@if, @for) instead of structural directives (*ngIf, *ngFor). +- **Signals**: Prefer signals over RxJS for simpler state management within components. ### TypeScript Enhancements - **Strict Type Checking**: Enable strict mode (`strict: true`) and avoid excessive use of `any`. - **Interfaces vs. Types**: Prefer interfaces for object definitions and use type aliases for unions and intersections. - **Generics**: Use meaningful type parameter names and constrain generics when applicable. -- **Documentation**: Employ JSDoc comments functions and generic parameters to improve code clarity. +- **Documentation**: Employ JSDoc comments for functions and generic parameters to improve code clarity. +- **Non-Nullability**: Use the non-null assertion operator (!) sparingly and only when you're certain a value cannot be null. +- **Type Guards**: Implement custom type guards to handle type narrowing safely. +- **Immutability**: Favor immutable data structures and use readonly modifiers when applicable. +- **Exhaustiveness Checking**: Use exhaustiveness checking for switch statements handling union types. ## TypeScript Guidelines @@ -48,18 +236,18 @@ This section extends the core code style principles with Angular-specific and ad - Prefer `interface` over `type` for object definitions - Use `type` for unions, intersections, and mapped types - - Follow Angular's naming convention: `IComponentProps` for props interfaces + - Follow Angular's naming convention: Don't prefix interfaces with 'I' (use `ComponentProps` not `IComponentProps`) - Extend interfaces instead of repeating properties - Use readonly modifiers where appropriate ```typescript // Good - interface IBaseProps { + interface BaseProps { readonly id: string; name: string; } - interface IUserProps extends IBaseProps { + interface UserProps extends BaseProps { email: string; } @@ -75,9 +263,49 @@ This section extends the core code style principles with Angular-specific and ad - **Enums and Constants**: - - Use `const enum` for better performance - - Only use regular `enum` when runtime access is required - - Prefer union types for simple string literals + - Prefer this order of implementation (from most to least preferred): + + 1. `const enum` for better compile-time performance + 2. Object literals with `as const` for runtime flexibility + 3. Regular `enum` only when necessary for runtime access + + - **When to use each approach**: + - Use `const enum` for internal application enumerations that don't need runtime access + - Use `const object as const` when values need to be inspected at runtime or exported in an API + - Use regular `enum` only when runtime enumeration object access is required + + ```typescript + // Good - const enum (preferred for most cases) + // Advantages: Tree-shakable, type-safe, disappears at compile time + export const enum ConstEnumStates { + NotSet = 'not-set', + Success = 'success', + } + + // Good - const object with 'as const' assertion + // Advantages: Runtime accessible, works well with API boundaries + export const ConstStates = { + NotSet: 'not-set', + Success: 'success', + } as const; + + // Types can be extracted from const objects + type ConstStatesType = (typeof ConstStates)[keyof typeof ConstStates]; + + // Least preferred - regular enum + // Only use when you need the enum object at runtime + export enum States { + NotSet = 'not-set', + Success = 'success', + } + ``` + + - Use union types as an alternative for simple string literals + + ```typescript + // Alternative approach using union types + export type StatusType = 'not-set' | 'success'; + ``` - **Functions and Methods**: @@ -94,7 +322,7 @@ This section extends the core code style principles with Angular-specific and ad * @param id - The user's unique identifier * @param includeDetails - Whether to include additional user details */ - const getUser = (id: string, includeDetails = false): Promise => { + const getUser = (id: string, includeDetails = false): Promise => { // ...implementation }; @@ -113,12 +341,12 @@ Example: ```typescript // Good -interface IUserProps { +interface UserProps { id: string; name: string; } -interface IAdminProps extends IUserProps { +interface AdminProps extends UserProps { permissions: string[]; } @@ -127,7 +355,7 @@ const enum UserRole { User = 'USER', } -const getUser = (id: string): Promise => { +const getUser = (id: string): Promise => { // ...implementation }; @@ -170,20 +398,116 @@ function getUser(id) { subscription: Subscription; ngOnInit() { - this.subscription = this.userService.getUsers().subscribe((users) => (this.users = users)); + this.subscription = this.userService + .getUsers() + .subscribe((users) => (this.users = users)); } } ``` -- **Templates** +- **Templates and Control Flow**: - - Use new control flow syntax - instead if \*ngIf use the @if syntax + - Use modern control flow syntax (`@if`, `@for`, `@switch`) instead of structural directives (`*ngIf`, `*ngFor`, `*ngSwitch`). + + ```html + +
+ @if (user) { +

Welcome, {{ user.name }}!

+ } @else if (isLoading) { +

Loading user data...

+ } @else { +

Please log in

+ } + +
    + @for (item of items; track item.id) { +
  • {{ item.name }}
  • + } @empty { +
  • No items available
  • + } +
+ + @switch (userRole) { @case ('admin') { + + } @case ('manager') { + + } @default { + + } } +
+ + +
+

Welcome, {{ user.name }}!

+

Loading user data...

+

Please log in

+ +
    +
  • {{ item.name }}
  • +
  • No items available
  • +
+ + + + +
+ ``` + + - When using `@for`, always specify the `track` expression to optimize rendering performance: + - Use a unique identifier property (like `id` or `uuid`) when available + - Only use `$index` for static collections that never change + - Avoid using non-unique properties that could result in DOM mismatches + - Leverage contextual variables in `@for` blocks: + - `$index` - Current item index + - `$first` - Boolean indicating if this is the first item + - `$last` - Boolean indicating if this is the last item + - `$even` - Boolean indicating if this index is even + - `$odd` - Boolean indicating if this index is odd + - `$count` - Total number of items in the collection + + ```html + + @for (item of items; track item.id; let i = $index, isLast = $last) { +
  • {{ i + 1 }}. {{ item.name }}
  • + } + ``` + + - Use the `@empty` block with `@for` to handle empty collections gracefully + - Store conditional expression results in variables for clearer templates: + + ```html + + @if (user.permissions.canEditSettings; as canEdit) { + + } + ``` ## Project-Specific Preferences -- **Frameworks**: Follow best practices for Nx, Hono, and Zod. +- **Frameworks**: Follow best practices for Nx, Angular, date-fns, Ngrx, RxJs and Zod. - **Testing**: Use Jest with Spectator for unit tests and follow the Arrange-Act-Assert pattern. -- **File Naming**: Use kebab-case for filenames (e.g., `my-component.ts`). +- **File Naming**: + + - Use kebab-case for filenames (e.g., `my-component.ts`). + - Follow a pattern that describes the symbol's feature then its type: `feature.type.ts` + + ``` + // Good examples + user.service.ts + auth.guard.ts + product-list.component.ts + order.model.ts + + // Bad examples + service-user.ts + userService.ts + ``` + - **Comments**: Use JSDoc for documenting functions, classes, and modules. ## Formatting @@ -198,25 +522,10 @@ function getUser(id) { - Use ESLint with the recommended TypeScript and Nx configurations. - Prettier should be used for consistent formatting. -## Example - -```typescript -// Good Example -interface User { - id: string; - name: string; -} - -const getUser = (id: string): User => { - // ...function logic... -}; - -// Bad Example -function getUser(id) { - // ...function logic... -} -``` - ## References -- [Angular Style Guide](https://angular.dev/style-guide#) +- [Angular Style Guide](https://angular.dev/style-guide) - Official Angular style guide with best practices for Angular development +- [Angular Control Flow](https://angular.dev/guide/templates/control-flow) - Official Angular documentation on the new control flow syntax (@if, @for, @switch) +- [TypeScript Style Guide](https://ts.dev/style/) - TypeScript community style guide with patterns and practices +- [SOLID Design Principles](https://en.wikipedia.org/wiki/SOLID) - Wikipedia article explaining the SOLID principles in object-oriented design +- [Clean Code](https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882) - Robert C. Martin's seminal book on writing clean, maintainable code diff --git a/libs/oms/feature/return-search/src/lib/return-search-result/return-search-result.component.html b/libs/oms/feature/return-search/src/lib/return-search-result/return-search-result.component.html index 1cd2e49d9..0fd29388b 100644 --- a/libs/oms/feature/return-search/src/lib/return-search-result/return-search-result.component.html +++ b/libs/oms/feature/return-search/src/lib/return-search-result/return-search-result.component.html @@ -12,17 +12,28 @@ [rollbackOnClose]="true" > - @if (showOrderByToolbar()) { - + @if (mobileBreakpoint()) { + } @else { - + } +@if (mobileBreakpoint() && showOrderByToolbarMobile()) { + +} + {{ entityHits() }} Einträge @@ -40,7 +51,10 @@ } @placeholder {
    - +
    } } diff --git a/libs/oms/feature/return-search/src/lib/return-search-result/return-search-result.component.ts b/libs/oms/feature/return-search/src/lib/return-search-result/return-search-result.component.ts index aaf83f2ab..1dcc25a2f 100644 --- a/libs/oms/feature/return-search/src/lib/return-search-result/return-search-result.component.ts +++ b/libs/oms/feature/return-search/src/lib/return-search-result/return-search-result.component.ts @@ -4,7 +4,7 @@ import { Component, computed, inject, - signal, + linkedSignal, } from '@angular/core'; import { injectActivatedProcessId } from '@isa/core/process'; @@ -14,20 +14,33 @@ import { FilterService, SearchBarInputComponent, OrderByToolbarComponent, - OrderByDropdownComponent, } from '@isa/shared/filter'; import { IconButtonComponent } from '@isa/ui/buttons'; import { EmptyStateComponent } from '@isa/ui/empty-state'; -import { provideIcons } from '@ng-icons/core'; -import { isaActionSort } from '@isa/icons'; -import { ReceiptListItem, ReturnSearchStatus, ReturnSearchStore } from '@isa/oms/data-access'; +import { NgIconComponent, provideIcons } from '@ng-icons/core'; +import { isaActionSort, isaActionFilter } from '@isa/icons'; +import { + ReceiptListItem, + ReturnSearchStatus, + ReturnSearchStore, +} from '@isa/oms/data-access'; import { ReturnSearchResultItemComponent } from './return-search-result-item/return-search-result-item.component'; -import { Breakpoint, BreakpointDirective, InViewportDirective } from '@isa/ui/layout'; +import { Breakpoint, InViewportDirective } from '@isa/ui/layout'; import { CallbackResult, ListResponseArgs } from '@isa/common/result'; import { injectRestoreScrollPosition } from '@isa/utils/scroll-position'; import { breakpoint } from '@isa/ui/layout'; +/** + * Component responsible for displaying return search results. + * + * This component handles: + * - Displaying a list of return search results + * - Filtering and sorting results + * - Searching for returns + * - Pagination with infinite scrolling + * - Responsive layout changes based on device size + */ @Component({ selector: 'oms-feature-return-search-result', templateUrl: './return-search-result.component.html', @@ -37,30 +50,50 @@ import { breakpoint } from '@isa/ui/layout'; RouterLink, ReturnSearchResultItemComponent, OrderByToolbarComponent, - OrderByDropdownComponent, IconButtonComponent, SearchBarInputComponent, EmptyStateComponent, FilterMenuButtonComponent, - BreakpointDirective, InViewportDirective, + NgIconComponent, ], - providers: [provideIcons({ isaActionSort })], + providers: [provideIcons({ isaActionSort, isaActionFilter })], }) export class ReturnSearchResultComponent implements AfterViewInit { - showOrderByToolbar = breakpoint([Breakpoint.DekstopL, Breakpoint.DekstopXL]); - + /** Route service for navigation and route information */ #route = inject(ActivatedRoute); + + /** Router service for programmatic navigation */ #router = inject(Router); + + /** Service for managing filters and search queries */ #filterService = inject(FilterService); + /** Utility for restoring scroll position when returning to this view */ restoreScrollPosition = injectRestoreScrollPosition(); + /** Current process ID from the activated route */ processId = injectActivatedProcessId(); + + /** Store for managing return search data and operations */ returnSearchStore = inject(ReturnSearchStore); + /** Enum reference for template usage */ ReturnSearchStatus = ReturnSearchStatus; + /** Signal tracking whether the viewport is at tablet size or above */ + mobileBreakpoint = breakpoint([Breakpoint.Tablet]); + + /** + * Signal controlling the visibility of the order-by toolbar on mobile + * Initially shows toolbar when NOT on mobile + */ + showOrderByToolbarMobile = linkedSignal(() => !this.mobileBreakpoint()); + + /** + * Computes the current return search entity based on the active process ID + * @returns The return search entity or undefined if no process ID is available + */ entity = computed(() => { const processId = this.processId(); if (processId) { @@ -69,45 +102,83 @@ export class ReturnSearchResultComponent implements AfterViewInit { return undefined; }); + /** + * Returns the list of return items from the current entity + * @returns Array of return items or empty array if none available + */ entityItems = computed(() => { return this.entity()?.items ?? []; }); + /** + * Returns the total number of hits from the search results + * @returns Total hits or 0 if no data available + */ entityHits = computed(() => { return this.entity()?.hits ?? 0; }); + /** + * Returns the current status of the search operation + * @returns Current status or Idle if no entity is available + */ entityStatus = computed(() => { return this.entity()?.status ?? ReturnSearchStatus.Idle; }); + /** + * Determines whether to render the item list based on available items + * @returns Boolean indicating if items are available to display + */ renderItemList = computed(() => { return this.entityItems().length; }); + /** + * Determines whether to show pagination loading indicator + * @returns Boolean indicating if pagination loading should be shown + */ renderPagingLoader = computed(() => { return this.entityStatus() === ReturnSearchStatus.Pending; }); + /** + * Determines whether to show the main search loading indicator + * Shows loader only when search is pending and no items are available yet + * @returns Boolean indicating if search loading should be shown + */ renderSearchLoader = computed(() => { - return this.entityStatus() === ReturnSearchStatus.Pending && this.entityItems().length === 0; + return ( + this.entityStatus() === ReturnSearchStatus.Pending && + this.entityItems().length === 0 + ); }); + /** + * Determines whether to render the page trigger for infinite scrolling + * Triggers pagination when more results are available than currently loaded + * @returns Boolean indicating if page trigger should be shown + */ renderPageTrigger = computed(() => { const entity = this.entity(); - if (!entity) return false; - if (entity.status === ReturnSearchStatus.Pending) return false; + if (!entity || entity.status === ReturnSearchStatus.Pending) return false; const { hits, items } = entity; - if (!hits || !Array.isArray(items)) return false; - - return hits > items.length; + return Boolean(hits && Array.isArray(items) && hits > items.length); }); + /** + * Lifecycle hook called after the component's view has been initialized + * Restores scroll position when returning to this view + */ ngAfterViewInit(): void { this.restoreScrollPosition(); } + /** + * Initiates a search operation with the current filter settings + * Navigates directly to the receipt if only one result is found + */ search() { const processId = this.processId(); if (processId) { @@ -121,6 +192,11 @@ export class ReturnSearchResultComponent implements AfterViewInit { } } + /** + * Callback function for search operations + * Automatically navigates to the receipt detail view if exactly one result is found + * @param result The callback result containing search data + */ searchCb = ({ data }: CallbackResult>) => { if (data) { if (data.result.length === 1) { @@ -129,6 +205,11 @@ export class ReturnSearchResultComponent implements AfterViewInit { } }; + /** + * Handles infinite scrolling pagination when the page trigger enters the viewport + * Loads more results when triggered + * @param inViewport Boolean indicating if the trigger element is in viewport + */ paging(inViewport: boolean) { if (!inViewport) { return; @@ -144,6 +225,10 @@ export class ReturnSearchResultComponent implements AfterViewInit { } } + /** + * Navigates to a specified path while preserving filter query parameters + * @param path Array of path segments for navigation + */ navigate(path: (string | number)[]) { this.#router.navigate(path, { relativeTo: this.#route, diff --git a/libs/shared/filter/src/lib/order-by/index.ts b/libs/shared/filter/src/lib/order-by/index.ts index 88b0a81af..bf7d5138a 100644 --- a/libs/shared/filter/src/lib/order-by/index.ts +++ b/libs/shared/filter/src/lib/order-by/index.ts @@ -1,2 +1 @@ export * from './order-by-toolbar.component'; -export * from './order-by-dropdown.component'; diff --git a/libs/shared/filter/src/lib/order-by/order-by-dropdown.component.html b/libs/shared/filter/src/lib/order-by/order-by-dropdown.component.html deleted file mode 100644 index 387ceccfb..000000000 --- a/libs/shared/filter/src/lib/order-by/order-by-dropdown.component.html +++ /dev/null @@ -1,18 +0,0 @@ - - @for (option of orderByOptions(); track option.by + option.dir) { - -
    {{ option.label }}
    -
    - -
    -
    - } -
    diff --git a/libs/shared/filter/src/lib/order-by/order-by-dropdown.component.scss b/libs/shared/filter/src/lib/order-by/order-by-dropdown.component.scss deleted file mode 100644 index a1aee2a49..000000000 --- a/libs/shared/filter/src/lib/order-by/order-by-dropdown.component.scss +++ /dev/null @@ -1,3 +0,0 @@ -:host { - @apply inline-flex; -} diff --git a/libs/shared/filter/src/lib/order-by/order-by-dropdown.component.ts b/libs/shared/filter/src/lib/order-by/order-by-dropdown.component.ts deleted file mode 100644 index 4080535dd..000000000 --- a/libs/shared/filter/src/lib/order-by/order-by-dropdown.component.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { ChangeDetectionStrategy, Component, computed, inject } from '@angular/core'; -import { DropdownButtonComponent, DropdownOptionComponent } from '@isa/ui/input-controls'; -import { FilterService, OrderByOption } from '../core'; -import { FormsModule } from '@angular/forms'; -import { NgIconComponent, provideIcons } from '@ng-icons/core'; -import { isaSortByDownMedium, isaSortByUpMedium } from '@isa/icons'; - -@Component({ - selector: 'filter-order-by-dropdown', - templateUrl: './order-by-dropdown.component.html', - styleUrls: ['./order-by-dropdown.component.scss'], - changeDetection: ChangeDetectionStrategy.OnPush, - imports: [DropdownButtonComponent, DropdownOptionComponent, FormsModule, NgIconComponent], - providers: [provideIcons({ desc: isaSortByDownMedium, asc: isaSortByUpMedium })], -}) -export class OrderByDropdownComponent { - #filter = inject(FilterService); - - orderByOptions = this.#filter.orderBy; - - selectedOrderBy = computed(() => this.orderByOptions().find((o) => o.selected)); - - setOrderBy(option: OrderByOption) { - this.#filter.setOrderBy(option.by, option.dir); - } -}