feat(ui): create meeting form with updated ux #45
Conversation
- Add lfx-stepper wrapper component with value, linear, and transitionOptions inputs - Add lfx-step helper component with value and disabled inputs - Add lfx-step-panel helper component for stepper content panels - Follow established wrapper component patterns with proper signal usage - Components support content projection and maintain PrimeNG compatibility - All components include proper TypeScript interfaces and event handling - Add license headers to all template files Implements LFXV2-279: Create PrimeNG Stepper wrapper component 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
Add routerLink navigation support to lfx-button component for cleaner navigation implementation. - Add routerLink, routerLinkActive, and routerLinkActiveOptions inputs to lfx-button component - Update button template to conditionally handle routerLink navigation vs onClick events - Remove unused onCreateMeeting method from meeting dashboard component - Update meeting dashboard "Schedule First Meeting" button to use routerLink navigation - Clean up unused imports (MeetingFormComponent, Injector) - Fix form imports in meeting-create component (moved from @angular/core to @angular/forms) LFXV2-280 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
- Create meeting type selection component with 6 meeting types - Fix PrimeNG stepper dependency injection issues by using direct components - Implement reactive form validation with signal-based state management - Add proper form valueChanges subscription for real-time validation - Update stepper to use subscription-based canProceed validation instead of computed - Style meeting type cards with color-coded icons matching committee themes - Add private meeting toggle with proper form integration - Implement working navigation between stepper steps - Add comprehensive form validation for meeting type step LFXV2-281 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Asitha de Silva <[email protected]>
- Add complete meeting details component with auto-title generation - Implement AI agenda helper with gradient UI and mock generation - Add recurring meeting toggle with dynamic recurrence options - Include scroll-to-top functionality for smooth step navigation - Update meeting type selection to use ngClass for better performance - Add proper form integration with reactive patterns using subscriptions Implements LFXV2-282 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a routed, multi-step Meeting Create wizard under project/:slug, plus MeetingDetails and MeetingTypeSelection components. Replaces dialog-based creation in MeetingDashboard with router navigation. Extends shared ButtonComponent to accept routerLink and supports router-driven navigation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MeetingDashboard
participant Router
participant MeetingCreateComponent
participant ProjectService
participant MeetingService
participant MessageService
User->>MeetingDashboard: Click "Schedule Meeting"
MeetingDashboard->>Router: Navigate to /project/:slug/meetings/create
Router->>MeetingCreateComponent: Instantiate component
MeetingCreateComponent->>ProjectService: Fetch current project
User->>MeetingCreateComponent: Complete steps & submit
MeetingCreateComponent->>MeetingService: createMeeting(request)
MeetingService-->>MeetingCreateComponent: Success / Error
alt Success
MeetingCreateComponent->>MessageService: Show success
MeetingCreateComponent->>Router: Navigate to meetings list
else Error
MeetingCreateComponent->>MessageService: Show error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR implements a new meeting creation form with a multi-step stepper interface to replace the previous single-page form. The design improves UX through a guided workflow with validation between steps, auto-title generation, AI agenda assistance, and enhanced form components.
Key changes:
- Created a multi-step stepper interface with 5 steps (2 implemented, 3 placeholders)
- Added meeting type selection with visual cards and meeting type information
- Implemented comprehensive meeting details form with AI agenda helper and recurring meeting options
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| stepper/stepper.component.ts | Generic stepper wrapper component for PrimeNG stepper functionality |
| stepper/stepper.component.html | Template for stepper with step list and panels content projection |
| step/step.component.ts | Individual step component wrapper for stepper navigation |
| step/step.component.html | Template for step with content projection |
| step-panel/step-panel.component.ts | Step panel wrapper component for stepper content areas |
| step-panel/step-panel.component.html | Template for step panel with content projection |
| button/button.component.ts | Added router navigation support to existing button component |
| button/button.component.html | Added routerLink property binding to button template |
| meeting-dashboard.component.ts | Replaced modal-based meeting creation with navigation to new route |
| meeting-dashboard.component.html | Updated button to use router navigation instead of click handler |
| meeting-type-selection.component.ts | New component for visual meeting type selection with form integration |
| meeting-type-selection.component.html | Template with meeting type cards and private meeting toggle |
| meeting-details.component.ts | Comprehensive form component with AI helper, validation, and recurrence logic |
| meeting-details.component.html | Detailed form template with date/time controls and AI agenda generator |
| meeting-create.component.ts | Main stepper orchestration component with form management and validation |
| meeting-create.component.html | Stepper layout with navigation and step content areas |
| app.routes.ts | Added new route for meeting creation form |
Comments suppressed due to low confidence (2)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts:34
- The
totalStepsconstant is defined inMeetingDetailsComponentbut appears to be unrelated to this component's functionality. This should likely be in the parentMeetingCreateComponentinstead.
})
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts:281
- [nitpick] In the mock agenda for MAINTAINERS meeting type, 'Maintainer Sync Agenda' should be 'Maintainers Sync Agenda' to match the enum naming convention.
4. **Action Planning** (5 min)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.ts
Outdated
Show resolved
Hide resolved
...x-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
Show resolved
Hide resolved
...x-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
Show resolved
Hide resolved
- Remove unused PrimeNG stepper wrapper component - Replace manual subscription management with takeUntilDestroyed operator - Inject DestroyRef for proper subscription cleanup in Angular 16+ pattern - Fix memory leaks in meeting-details and meeting-create components Signed-off-by: Asitha de Silva <[email protected]>
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
- Remove step-panel and step components that were part of stepper wrapper - Complete cleanup of unused PrimeNG wrapper components Signed-off-by: Asitha de Silva <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (26)
apps/lfx-pcc/src/app/shared/components/button/button.component.ts (1)
52-54: Consider exposing common RouterLink extras (queryParams, fragment, state) for flexibilityForwarding a few optional router navigation extras now can prevent sprinkling ad-hoc wrappers later. If you agree, I can follow up with a small change to add inputs and bind them through to p-button.
Example additions (TS):
public readonly queryParams = input<Record<string, any> | undefined>(undefined); public readonly fragment = input<string | undefined>(undefined); public readonly state = input<Record<string, any> | undefined>(undefined);And in the template:
[preserveFragment]="false" [queryParams]="queryParams()" [fragment]="fragment()" [state]="state()"apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.html (1)
4-11: Add data-testid attributes to the shared Stepper wrapper.Aligns with testing guidelines and makes selectors stable.
-<p-stepper [value]="value() ?? 0" [linear]="linear()" [style]="style()" (valueChange)="handleValueChange($event)"> - <p-step-list> +<p-stepper [value]="value() ?? 0" [linear]="linear()" [style]="style()" (valueChange)="handleValueChange($event)" data-testid="shared-stepper-root"> + <p-step-list data-testid="shared-stepper-list"> <ng-content select="lfx-step"></ng-content> </p-step-list> - <p-step-panels> + <p-step-panels data-testid="shared-stepper-panels"> <ng-content select="lfx-step-panel"></ng-content> </p-step-panels> </p-stepper>apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.html (1)
16-63: Consider using the shared Stepper wrappers (lfx-stepper,lfx-step,lfx-step-panel) for consistency.You introduced shared wrappers; this template uses PrimeNG directly. Migrating to the wrappers keeps API uniform and eases future styling/test hooks.
If desired, I can provide a concrete replacement snippet aligned with the wrapper APIs used elsewhere.
apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (1)
261-267: Good shift to router-based creation; disable the item when no project and avoid'#'.Using
'#'can still change the URL fragment unexpectedly. Prefer disabling the item and omitting the routerLink when the project is not available. Also consider updating menu items when theprojectsignal resolves asynchronously.- const project = this.project(); + const project = this.project(); return [ { label: 'Schedule Meeting', icon: 'fa-light fa-calendar-plus text-sm', - routerLink: project ? `/project/${project.slug}/meetings/create` : '#', + routerLink: project ? `/project/${project.slug}/meetings/create` : undefined, + disabled: !project, },Optional: ensure menu items react to
project()changes (signals). For example, add in the constructor:// In constructor (requires: import { effect } from '@angular/core';) effect(() => { // Recompute when project() changes this.menuItems = this.initializeMenuItems(); });apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.html (2)
4-4: Add a root data-testid for the component container.Improves test stability and aligns with test-id guidelines.
-<div class="space-y-6"> +<div class="space-y-6" data-testid="meeting-type-selection-root">
28-39: Improve keyboard and ARIA accessibility for selectable cards and add a grid test id.Treat the options as a radio group so users can navigate/choose with keyboard. Adds
role,aria-checked, and key handlers; also adds a test id to the grid.- <div class="grid grid-cols-1 md:grid-cols-2 gap-3"> + <div class="grid grid-cols-1 md:grid-cols-2 gap-3" role="radiogroup" aria-label="Meeting type" data-testid="meeting-type-grid"> @for (option of meetingTypeOptions; track option.value) { @let typeInfo = getMeetingTypeInfo(option.value); <div class="p-4 border rounded-lg cursor-pointer transition-all hover:border-blue-600 hover:bg-blue-50" + role="radio" + tabindex="0" + [attr.aria-checked]="form().get('meeting_type')?.value === option.value" + (keydown.enter)="onMeetingTypeSelect(option.value)" + (keydown.space)="onMeetingTypeSelect(option.value); $event.preventDefault()" [ngClass]="{ 'border-blue-600 bg-blue-50': form().get('meeting_type')?.value === option.value, 'border-slate-200 bg-white': form().get('meeting_type')?.value !== option.value, }" (click)="onMeetingTypeSelect(option.value)" [attr.data-testid]="'meeting-type-option-' + option.value.toLowerCase()">apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.ts (2)
5-13: Adopt OnPush change detection for performanceThese wrapper components are pure/presentational. OnPush avoids unnecessary CD churn in large forms/steppers.
Apply:
-import { Component, ContentChild, input, output, TemplateRef } from '@angular/core'; +import { Component, ContentChild, input, output, TemplateRef, ChangeDetectionStrategy } from '@angular/core'; @Component({ selector: 'lfx-step-panel', standalone: true, imports: [CommonModule, StepperModule], templateUrl: './step-panel.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, })
16-16: Prefer TemplateRef over anyTighten typing; avoids any leakage while remaining flexible.
- @ContentChild('contentTemplate', { static: false, descendants: false }) public contentTemplate?: TemplateRef<any>; + @ContentChild('contentTemplate', { static: false, descendants: false }) public contentTemplate?: TemplateRef<unknown>;apps/lfx-pcc/src/app/shared/components/step/step.component.ts (3)
5-13: Adopt OnPush change detection for performanceSame reasoning as the panel; this will keep the wizard snappy under heavy form activity.
-import { Component, ContentChild, input, output, TemplateRef } from '@angular/core'; +import { Component, ContentChild, input, output, TemplateRef, ChangeDetectionStrategy } from '@angular/core'; @Component({ selector: 'lfx-step', standalone: true, imports: [CommonModule, StepperModule], templateUrl: './step.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, })
16-16: Prefer TemplateRef over anyMinor typing improvement.
- @ContentChild('content', { static: false, descendants: false }) public contentTemplate?: TemplateRef<any>; + @ContentChild('content', { static: false, descendants: false }) public contentTemplate?: TemplateRef<unknown>;
14-29: Naming consistency for projected template tokenStep uses 'content' while StepPanel uses 'contentTemplate' as the slot token. Consider aligning for consistency to lower cognitive load when authoring templates.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (3)
29-36: Type meetingTypeOptions explicitly; avoid implicit unionsAdd a small option interface and type the array as readonly to improve maintainability and IntelliSense.
interface MeetingTypeInfo { icon: string; description: string; examples: string; color: string; } +interface MeetingTypeOption { + label: string; + value: MeetingType; +} @@ - public readonly meetingTypeOptions = [ + public readonly meetingTypeOptions: readonly MeetingTypeOption[] = [ { label: 'Board', value: MeetingType.BOARD }, { label: 'Maintainers', value: MeetingType.MAINTAINERS }, { label: 'Marketing', value: MeetingType.MARKETING }, { label: 'Technical', value: MeetingType.TECHNICAL }, { label: 'Legal', value: MeetingType.LEGAL }, { label: 'Other', value: MeetingType.OTHER }, ];
90-93: Mark as dirty and update validity on selectionEnsures validators/UI state update immediately and form persistence catches the change.
- public onMeetingTypeSelect(meetingType: MeetingType): void { - this.form().get('meeting_type')?.setValue(meetingType); - this.form().get('meeting_type')?.markAsTouched(); - } + public onMeetingTypeSelect(meetingType: MeetingType): void { + const control = this.form().get('meeting_type'); + control?.setValue(meetingType, { emitEvent: true }); + control?.markAsDirty(); + control?.markAsTouched(); + control?.updateValueAndValidity(); + }
4-10: Consider avoiding direct PrimeNG TooltipModule importsTeam learning says PrimeNG should be wrapped in LFX components for UI library independence. If there’s an LFX tooltip wrapper, prefer that over importing TooltipModule here.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html (4)
241-246: Use LFX tooltip wrapper instead of pTooltip (PrimeNG) if availableDirect pTooltip usage couples the template to PrimeNG. To keep UI lib independence, consider an LFX tooltip directive/component wrapper.
Would you like a lightweight LFXTooltipDirective scaffold you can reuse?
261-268: Optional: add data-testid to info icons for E2E reliabilityAdding data-testid to the info icons (recurrence/early-join tooltips) can make E2E targeting more robust.
Example ids:
- meeting-details-recurrence-info
- meeting-details-early-join-info
18-21: Avoid calling methods in hot templates where possibleFunctions like titleWasAutoGenerated(), showAiHelper(), isGeneratingAgenda(), minDate(), isCustomDuration(), recurrenceOptions() can be exposed as signals/computed or readonly properties to reduce CD invocations.
Also applies to: 42-84, 101-206, 221-257, 259-285
63-73: Guard against non-string aiPrompt values (minor)The optional chain covers undefined, but if aiPrompt can be non-string, using String(value).trim() in the component before binding would be more explicit.
apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts (1)
5-13: Adopt OnPush change detection for performanceRecommended for wrapper components to minimize CD cost in multi-step flows.
-import { Component, input, output } from '@angular/core'; +import { Component, input, output, ChangeDetectionStrategy } from '@angular/core'; @@ @Component({ selector: 'lfx-stepper', standalone: true, imports: [CommonModule, StepperModule], templateUrl: './stepper.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, })apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts (5)
51-56: Avoid subscription leaks: use takeUntilDestroyed for valueChanges subscriptionThe constructor subscribes to form.valueChanges without teardown. Prefer Angular’s takeUntilDestroyed to auto-clean up when the component is destroyed.
Apply:
@@ -import { Component, computed, effect, inject, signal } from '@angular/core'; +import { Component, computed, effect, inject, signal } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; @@ public constructor() { // Subscribe to form value changes and update validation signals - this.form().valueChanges.subscribe(() => { + this.form().valueChanges.pipe(takeUntilDestroyed()).subscribe(() => { this.updateCanProceed(); });
97-104: Simplify: mark all controls touched via markAllAsTouched()Iterating over controls is unnecessary and misses nested controls. Use FormGroup.markAllAsTouched() and update validity.
public onSubmit(): void { // Mark all form controls as touched to show validation errors - Object.keys(this.form().controls).forEach((key) => { - const control = this.form().get(key); - control?.markAsTouched(); - control?.markAsDirty(); - }); + this.form().markAllAsTouched(); + this.form().updateValueAndValidity();
153-171: Ensure submitting state resets on both success and errorCurrently submitting is reset only on error; on success you navigate away but, for safety and consistency, use finalize.
- this.meetingService.createMeeting(meetingData).subscribe({ + this.meetingService + .createMeeting(meetingData) + .pipe(finalize(() => this.submitting.set(false))) + .subscribe({ next: () => { this.messageService.add({ severity: 'success', summary: 'Success', detail: 'Meeting created successfully', }); this.router.navigate(['/project', project.slug, 'meetings']); }, error: (error) => { console.error('Error creating meeting:', error); this.messageService.add({ severity: 'error', summary: 'Error', detail: 'Failed to create meeting. Please try again.', }); - this.submitting.set(false); }, - }); + });Add import:
import { finalize } from 'rxjs/operators';
446-456: Guard direct DOM/window access for SSR compatibilityIf this app uses server-side rendering at any point, direct window/document access will error. Add a platform check.
private scrollToStepper(): void { - // Find the meeting-create element and scroll to it minus 100px - const meetingCreate = document.getElementById('meeting-create'); - if (meetingCreate) { - const elementTop = meetingCreate.getBoundingClientRect().top + window.pageYOffset; - window.scrollTo({ - top: elementTop - 50, - behavior: 'smooth', - }); - } + // Find the meeting-create element and scroll to it (client-only) + if (typeof window === 'undefined' || typeof document === 'undefined') return; + const meetingCreate = document.getElementById('meeting-create'); + if (!meetingCreate) return; + const elementTop = meetingCreate.getBoundingClientRect().top + window.pageYOffset; + window.scrollTo({ top: elementTop - 50, behavior: 'smooth' }); }
344-371: combineDateTime should accept timezone (see calling site) and avoid returning '' silentlyTwo suggestions:
- Accept timezone to produce correct UTC ISO (see earlier comment).
- If the format is invalid, consider setting a control error rather than returning '' silently, which can mask issues downstream.
Example improvement (if you choose not to add a library), at minimum change signature to include timeZone?: string and pass it; then route invalid format to setErrors on startTime control.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (2)
139-154: Minor: resilient AI agenda generation UXConsider trimming and validating aiPrompt before setting isGeneratingAgenda, to avoid flicker; optionally disable the button while generating and handle errors with try/finally.
199-211: Auto-title: set the initial title on component init as wellIf both meeting_type and startDate are pre-populated (default start date/time), generateMeetingTitleIfNeeded should also run once during init, not only on subsequent changes.
Add at the end of ngOnInit():
this.generateMeetingTitleIfNeeded();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
apps/lfx-pcc/src/app/app.routes.ts(2 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts(2 hunks)apps/lfx-pcc/src/app/shared/components/button/button.component.html(1 hunks)apps/lfx-pcc/src/app/shared/components/button/button.component.ts(2 hunks)apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.html(1 hunks)apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.ts(1 hunks)apps/lfx-pcc/src/app/shared/components/step/step.component.html(1 hunks)apps/lfx-pcc/src/app/shared/components/step/step.component.ts(1 hunks)apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.html(1 hunks)apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/**/src/**/*.html
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/**/src/**/*.html: Always add data-testid attributes when creating new components for reliable test targeting
Use data-testid naming convention - '[section]-[component]-[element]' for hierarchical structure
Files:
apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.htmlapps/lfx-pcc/src/app/shared/components/stepper/stepper.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.htmlapps/lfx-pcc/src/app/shared/components/step/step.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.htmlapps/lfx-pcc/src/app/shared/components/button/button.component.htmlapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.html
{apps,packages}/**/*.{ts,tsx,js,jsx,css,scss}
📄 CodeRabbit Inference Engine (CLAUDE.md)
License headers are required on all source files
Files:
apps/lfx-pcc/src/app/shared/components/step/step.component.tsapps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.tsapps/lfx-pcc/src/app/shared/components/button/button.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.tsapps/lfx-pcc/src/app/app.routes.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
apps/lfx-pcc/src/app/shared/components/step/step.component.tsapps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.tsapps/lfx-pcc/src/app/shared/components/button/button.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.tsapps/lfx-pcc/src/app/app.routes.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-pcc/src/app/shared/components/step/step.component.tsapps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.tsapps/lfx-pcc/src/app/shared/components/button/button.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.tsapps/lfx-pcc/src/app/app.routes.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.tsapps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.tsapps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts
🧠 Learnings (1)
📚 Learning: 2025-08-07T16:35:57.360Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-pcc-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T16:35:57.360Z
Learning: All PrimeNG components are wrapped in LFX components for UI library independence
Applied to files:
apps/lfx-pcc/src/app/shared/components/button/button.component.tsapps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts
🧬 Code Graph Analysis (7)
apps/lfx-pcc/src/app/shared/components/step/step.component.ts (2)
apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.ts (1)
Component(8-28)apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts (1)
Component(8-42)
apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.ts (2)
apps/lfx-pcc/src/app/shared/components/step/step.component.ts (1)
Component(8-29)apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts (1)
Component(8-42)
apps/lfx-pcc/src/app/shared/components/button/button.component.ts (5)
apps/lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.ts (1)
Component(28-342)apps/lfx-pcc/src/app/shared/components/header/header.component.ts (1)
Component(24-145)apps/lfx-pcc/src/app/shared/components/breadcrumb/breadcrumb.component.ts (1)
Component(10-34)apps/lfx-pcc/src/app/layouts/project-layout/project-layout.component.ts (1)
Component(15-96)apps/lfx-pcc/src/app/app.component.ts (1)
Component(13-51)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts (4)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (1)
Component(18-94)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (1)
Component(18-344)packages/shared/src/interfaces/meeting.interface.ts (2)
CreateMeetingRequest(70-90)MeetingRecurrence(6-15)packages/shared/src/utils/timezone.utils.ts (1)
getUserTimezone(12-18)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (2)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (1)
Component(18-94)packages/shared/src/constants/timezones.ts (1)
TIMEZONES(10-95)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (2)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (1)
Component(18-344)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts (1)
Component(20-457)
apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts (2)
apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.ts (1)
Component(8-28)apps/lfx-pcc/src/app/shared/components/step/step.component.ts (1)
Component(8-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Code Quality Checks
🔇 Additional comments (13)
apps/lfx-pcc/src/app/shared/components/button/button.component.ts (1)
6-6: Router support in ButtonComponent looks goodImporting RouterModule and wiring it into the standalone component’s imports is correct and keeps the wrapper self-contained.
Also applies to: 13-13
apps/lfx-pcc/src/app/shared/components/button/button.component.html (1)
29-30: LGTM: RouterLink binding added to wrapper templateBinding [routerLink]="routerLink()" alongside the click handler is correct for enabling router-driven navigation without breaking existing onClick usage.
apps/lfx-pcc/src/app/app.routes.ts (1)
12-12: LGTM: Route added for meeting creation flowThe nested child route 'meetings/create' under 'project/:slug' is correct and aligns with the new wizard component.
Also applies to: 33-36
apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.ts (2)
14-28: Wrapper implementation looks cleanSignal inputs/outputs and content projection align with the Stepper usage. No functional concerns here.
8-13: Confirm data-testid attributes exist in step-panel templateNew components’ HTML should include data-testid hooks per guidelines.
Please ensure ./step-panel.component.html contains data-testid attributes for key root and interactive elements (e.g., panel container and navigation triggers).
apps/lfx-pcc/src/app/shared/components/step/step.component.ts (1)
18-21: Ensure disabled is wired in the templateYou expose disabled but we can’t see the template. Confirm it’s bound to the underlying PrimeNG step or equivalent.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (2)
11-16: Good: interface for MeetingTypeInfo (meets “prefer interfaces” guideline)Clear, simple, and reusable. No issues.
38-82: Nice: exhaustive mapping including MeetingType.NONEHaving NONE defined avoids undefined lookups and surprises. Fallback to OTHER in getMeetingTypeInfo is now mostly redundant but harmless.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html (1)
4-286: Comprehensive, test-friendly template — well doneGood coverage of data-testid hooks, validation surfaces, and UX affordances. Control-flow blocks are clean and readable.
apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts (1)
27-34: Public API is clearupdateValue and isStepActive helpers are handy for parent orchestration. No issues.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts (2)
1-3: License header presentHeader and SPDX identifier look good and satisfy repository guidelines.
125-131: Timezone handling missing when combining date and timeThe current
combineDateTime(date, time)always uses the browser’s local TZ, but the form also submits a separatetimezonefield—so users in other zones will see drift. To address this:• In both components, include
formValue.timezonewhen callingcombineDateTime:apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts - const startDateTime = this.combineDateTime(formValue.startDate, formValue.startTime); + const startDateTime = this.combineDateTime(formValue.startDate, formValue.startTime, formValue.timezone); apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-form/meeting-form.component.ts - const startDateTime = this.combineDateTime(formValue.startDate, formValue.startTime); + const startDateTime = this.combineDateTime(formValue.startDate, formValue.startTime, formValue.timezone);• Update
combineDateTimesignature to accept an optional IANA zone and convert accordingly (e.g. viadate-fns-tz’szonedTimeToUtc), so the ISO string aligns with the selected timezone.• If you’d rather not pull in
date-fns-tz, please confirm what the backend expects for:
start_time(UTC‐normalized ISO vs ISO+offset vs naive local string)- the
timezonefield (IANA name) and whether it drives server‐side parsingOnce we know the API semantics, we can finalize a minimal, correct implementation.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (1)
1-3: License header presentHeader and SPDX identifier look good and satisfy repository guidelines.
...pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.html
Outdated
Show resolved
Hide resolved
...x-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
Outdated
Show resolved
Hide resolved
...x-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
Show resolved
Hide resolved
...x-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
Show resolved
Hide resolved
...pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts
Outdated
Show resolved
Hide resolved
...pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts
Show resolved
Hide resolved
.../lfx-pcc/src/app/modules/project/meetings/meeting-dashboard/meeting-dashboard.component.html
Show resolved
Hide resolved
apps/lfx-pcc/src/app/shared/components/step-panel/step-panel.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-pcc/src/app/shared/components/step/step.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-pcc/src/app/shared/components/stepper/stepper.component.ts
Outdated
Show resolved
Hide resolved
- Add linear mode to prevent jumping ahead in stepper steps - Fix duration type handling to ensure numeric values for API - Add FormGroup-level futureDateTime validation to step validation - Replace brittle timezone validation with UTC timestamp comparison - Fix reactive recurrence options to update when start date changes - Add router navigation safety with slug guard - Add comprehensive data-testid attributes for testing - Initialize recurrence options properly on component load Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts (3)
218-220: Step 2 validation blocks progress, but the step content is a placeholderValidation checks meetingTool which isn’t implemented in the current UI, preventing navigation to later steps and eventual submission.
Short-term options:
- Temporarily bypass validation for step 2:
- case 2: // Platform & Features - return !!form.get('meetingTool')?.value; + case 2: // Platform & Features (placeholder) + return true;
- Or remove the required validator until the step ships:
- meetingTool: new FormControl('', [Validators.required]), + meetingTool: new FormControl(''),Add a TODO to re-enable once the feature is implemented.
35-37: Clarify step count while steps 3–5 are placeholdersHardcoding totalSteps to 5 is fine, but a note helps future maintainers until all steps are implemented.
public currentStep = signal<number>(0); - public readonly totalSteps = 5; + // NOTE: Steps 3–5 are placeholders pending implementation; keep in sync with getStepTitle/HTML. + public readonly totalSteps = 5;
309-347: Timezone comparison is brittle and can be wrong across locales/browsersConverting Date→localized string→Date is not spec-guaranteed and can miscompare. Compare UTC instants derived from the selected timezone.
Apply this diff to futureDateTimeValidator (and rely on a timezone-aware combineDateTime, see next comment):
private futureDateTimeValidator(): ValidatorFn { return (control: AbstractControl): ValidationErrors | null => { const formGroup = control as FormGroup; const startDate = formGroup.get('startDate')?.value; const startTime = formGroup.get('startTime')?.value; const timezone = formGroup.get('timezone')?.value; if (!startDate || !startTime || !timezone) { return null; // Don't validate if values are not set } - // Combine the date and time - const combinedDateTime = this.combineDateTime(startDate, startTime); + // Combine the date, time and timezone into a UTC instant + const combinedDateTime = this.combineDateTime(startDate, startTime, timezone); if (!combinedDateTime) { return null; // Invalid time format } - // Parse the combined datetime - const selectedDate = new Date(combinedDateTime); - - // Get current time in the selected timezone - const now = new Date(); - - // Create timezone-aware date strings for comparison - const selectedTimeString = selectedDate.toLocaleString('en-US', { timeZone: timezone }); - const currentTimeString = now.toLocaleString('en-US', { timeZone: timezone }); - - // Convert back to Date objects for comparison - const selectedTimeInZone = new Date(selectedTimeString); - const currentTimeInZone = new Date(currentTimeString); - - // Check if the selected time is in the future - if (selectedTimeInZone <= currentTimeInZone) { + // Compare UTC instants + const selectedUtc = new Date(combinedDateTime); + const nowUtc = new Date(); + if (selectedUtc <= nowUtc) { return { futureDateTime: true }; } return null; }; }
🧹 Nitpick comments (4)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html (2)
4-4: Add a root data-testid for the component containerTests will be more robust if the top-level wrapper has a stable handle.
Apply this diff:
-<div class="space-y-6"> +<div class="space-y-6" data-testid="meeting-details-root">
158-160: Surface futureDateTime error when timezone is touchedIf the user changes only the timezone (without touching date/time), the global futureDateTime error won’t show. Include timezone’s touched state.
-@if (form().errors?.['futureDateTime'] && (form().get('startDate')?.touched || form().get('startTime')?.touched)) { +@if ( + form().errors?.['futureDateTime'] && + (form().get('startDate')?.touched || form().get('startTime')?.touched || form().get('timezone')?.touched) +) {apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts (2)
126-128: Guard against invalid duration (NaN or non-positive) before submitValidators should catch most cases, but a defensive check prevents sending bad payloads (e.g., custom duration as empty string).
// Process duration value - ensure it's always a number - const duration = formValue.duration === 'custom' ? Number(formValue.customDuration) : Number(formValue.duration); + const duration = + formValue.duration === 'custom' ? Number(formValue.customDuration) : Number(formValue.duration); + if (!Number.isFinite(duration) || duration <= 0) { + this.messageService.add({ + severity: 'error', + summary: 'Invalid duration', + detail: 'Please provide a valid meeting duration.', + }); + this.submitting.set(false); + return; + }
451-462: Direct DOM access is fine for SPA, but consider SSR safetyIf this component is ever server-rendered, document/window access should be guarded. Not critical if SSR isn’t in scope.
// Example guard: if (typeof window !== 'undefined' && typeof document !== 'undefined') { // scroll logic }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html(1 hunks)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.html
- apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts
🧰 Additional context used
📓 Path-based instructions (4)
{apps,packages}/**/*.{ts,tsx,js,jsx,css,scss}
📄 CodeRabbit Inference Engine (CLAUDE.md)
License headers are required on all source files
Files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use TypeScript interfaces instead of union types for better maintainability
Files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
apps/**/src/**/*.html
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/**/src/**/*.html: Always add data-testid attributes when creating new components for reliable test targeting
Use data-testid naming convention - '[section]-[component]-[element]' for hierarchical structure
Files:
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html
🧬 Code Graph Analysis (1)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts (4)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (1)
Component(18-94)apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.ts (1)
Component(19-349)packages/shared/src/interfaces/meeting.interface.ts (2)
CreateMeetingRequest(70-90)MeetingRecurrence(6-15)packages/shared/src/utils/timezone.utils.ts (1)
getUserTimezone(12-18)
🔇 Additional comments (3)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-details/meeting-details.component.html (1)
26-41: Template bindings and test IDs look solidBindings map cleanly to the FormGroup controls, conditional blocks are cohesive, and the data-testid naming matches the required convention.
Also applies to: 42-84, 86-99, 101-121, 123-137, 139-155, 162-187, 189-205, 207-219, 221-234, 236-257, 259-284
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts (2)
136-155: Confirm meeting_type fallback valueYou default to 'None' when meeting_type is falsy. Ensure the backend accepts this literal (vs. the enum value, e.g., MeetingType.NONE) to avoid 400s or incorrect analytics.
If the API expects enum values, prefer importing MeetingType and using MeetingType.NONE instead of 'None'.
25-26: License header present and correctHeader complies with repository policy.
| import { getUserTimezone } from '@lfx-pcc/shared/utils'; | ||
| import { MeetingService } from '@services/meeting.service'; | ||
| import { ProjectService } from '@services/project.service'; | ||
| import { MessageService } from 'primeng/api'; | ||
| import { StepperModule } from 'primeng/stepper'; | ||
|
|
There was a problem hiding this comment.
Combine date/time ignores the selected timezone — start_time may be wrong
combineDateTime currently builds a local-time Date and returns toISOString(). If the user’s selected timezone differs from the browser timezone, the scheduled time will be off. Use a timezone-aware conversion to UTC and update call sites accordingly.
Apply these diffs:
- Import date-fns-tz helper:
import { MessageService } from 'primeng/api';
import { StepperModule } from 'primeng/stepper';
+import { zonedTimeToUtc } from 'date-fns-tz';- Use timezone when combining in onSubmit:
- const startDateTime = this.combineDateTime(formValue.startDate, formValue.startTime);
+ const startDateTime = this.combineDateTime(formValue.startDate, formValue.startTime, formValue.timezone);- Make combineDateTime timezone-aware:
- private combineDateTime(date: Date, time: string): string {
+ private combineDateTime(date: Date, time: string, timezone?: string): string {
if (!date || !time) return '';
// Parse the 12-hour format time (e.g., "12:45 AM" or "1:30 PM")
const match = time.match(/(\d{1,2}):(\d{2})\s*(AM|PM)/i);
if (!match) {
console.error('Invalid time format:', time);
return '';
}
let hours = parseInt(match[1], 10);
const minutes = parseInt(match[2], 10);
const period = match[3].toUpperCase();
// Convert to 24-hour format
if (period === 'PM' && hours !== 12) {
hours += 12;
} else if (period === 'AM' && hours === 12) {
hours = 0;
}
// Create a new date object with the selected date and time
- const combinedDate = new Date(date);
- combinedDate.setHours(hours, minutes, 0, 0);
-
- // Return ISO string
- return combinedDate.toISOString();
+ const local = new Date(date);
+ local.setHours(hours, minutes, 0, 0);
+
+ // If a timezone is provided, interpret the local wall-clock time in that timezone and convert to UTC
+ if (timezone) {
+ const utc = zonedTimeToUtc(local, timezone);
+ return utc.toISOString();
+ }
+
+ // Fallback: assume browser timezone (maintain current behavior)
+ return local.toISOString();
}Also applies to: 129-131, 349-376
| private getDefaultStartDateTime(): { date: Date; time: string } { | ||
| const now = new Date(); | ||
| // Add 1 hour to current time | ||
| now.setHours(now.getHours() + 1); | ||
|
|
||
| // Round up to next 15 minutes | ||
| const minutes = now.getMinutes(); | ||
| const roundedMinutes = Math.ceil(minutes / 15) * 15; | ||
| now.setMinutes(roundedMinutes); | ||
| now.setSeconds(0); | ||
| now.setMilliseconds(0); | ||
|
|
||
| // If rounding pushed us to next hour, adjust accordingly | ||
| if (roundedMinutes === 60) { | ||
| now.setHours(now.getHours() + 1); | ||
| now.setMinutes(0); | ||
| } | ||
|
|
||
| // Format time to 12-hour format (HH:MM AM/PM) | ||
| const hours = now.getHours(); | ||
| const mins = now.getMinutes(); | ||
| const period = hours >= 12 ? 'PM' : 'AM'; | ||
| let displayHours = hours > 12 ? hours - 12 : hours; | ||
| if (displayHours === 0) { | ||
| displayHours = 12; | ||
| } | ||
| const timeString = `${displayHours.toString().padStart(2, '0')}:${mins.toString().padStart(2, '0')} ${period}`; | ||
|
|
||
| return { | ||
| date: new Date(now), | ||
| time: timeString, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Bug: rounding to the next 15 minutes can jump an extra hour
setMinutes(60) already advances the hour; the subsequent block adds another hour, causing a double increment (e.g., 12:50 → 14:00). Fix the rounding logic.
Apply this diff:
private getDefaultStartDateTime(): { date: Date; time: string } {
const now = new Date();
// Add 1 hour to current time
now.setHours(now.getHours() + 1);
- // Round up to next 15 minutes
- const minutes = now.getMinutes();
- const roundedMinutes = Math.ceil(minutes / 15) * 15;
- now.setMinutes(roundedMinutes);
- now.setSeconds(0);
- now.setMilliseconds(0);
-
- // If rounding pushed us to next hour, adjust accordingly
- if (roundedMinutes === 60) {
- now.setHours(now.getHours() + 1);
- now.setMinutes(0);
- }
+ // Round up to the next 15 minutes without double-incrementing hours
+ const minutes = now.getMinutes();
+ const remainder = minutes % 15;
+ if (remainder !== 0) {
+ now.setMinutes(minutes + (15 - remainder));
+ }
+ now.setSeconds(0);
+ now.setMilliseconds(0);
// Format time to 12-hour format (HH:MM AM/PM)📝 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.
| private getDefaultStartDateTime(): { date: Date; time: string } { | |
| const now = new Date(); | |
| // Add 1 hour to current time | |
| now.setHours(now.getHours() + 1); | |
| // Round up to next 15 minutes | |
| const minutes = now.getMinutes(); | |
| const roundedMinutes = Math.ceil(minutes / 15) * 15; | |
| now.setMinutes(roundedMinutes); | |
| now.setSeconds(0); | |
| now.setMilliseconds(0); | |
| // If rounding pushed us to next hour, adjust accordingly | |
| if (roundedMinutes === 60) { | |
| now.setHours(now.getHours() + 1); | |
| now.setMinutes(0); | |
| } | |
| // Format time to 12-hour format (HH:MM AM/PM) | |
| const hours = now.getHours(); | |
| const mins = now.getMinutes(); | |
| const period = hours >= 12 ? 'PM' : 'AM'; | |
| let displayHours = hours > 12 ? hours - 12 : hours; | |
| if (displayHours === 0) { | |
| displayHours = 12; | |
| } | |
| const timeString = `${displayHours.toString().padStart(2, '0')}:${mins.toString().padStart(2, '0')} ${period}`; | |
| return { | |
| date: new Date(now), | |
| time: timeString, | |
| }; | |
| } | |
| private getDefaultStartDateTime(): { date: Date; time: string } { | |
| const now = new Date(); | |
| // Add 1 hour to current time | |
| now.setHours(now.getHours() + 1); | |
| // Round up to the next 15 minutes without double-incrementing hours | |
| const minutes = now.getMinutes(); | |
| const remainder = minutes % 15; | |
| if (remainder !== 0) { | |
| now.setMinutes(minutes + (15 - remainder)); | |
| } | |
| now.setSeconds(0); | |
| now.setMilliseconds(0); | |
| // Format time to 12-hour format (HH:MM AM/PM) | |
| const hours = now.getHours(); | |
| const mins = now.getMinutes(); | |
| const period = hours >= 12 ? 'PM' : 'AM'; | |
| let displayHours = hours > 12 ? hours - 12 : hours; | |
| if (displayHours === 0) { | |
| displayHours = 12; | |
| } | |
| const timeString = `${displayHours.toString().padStart(2, '0')}:${mins | |
| .toString() | |
| .padStart(2, '0')} ${period}`; | |
| return { | |
| date: new Date(now), | |
| time: timeString, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
around lines 275-307, the rounding logic sets minutes to 60 and then also
increments the hour again, causing a double-hour jump; change the flow so you
never call now.setMinutes(60) then add an extra hour — compute roundedMinutes
first and if roundedMinutes === 60, call now.setMinutes(0) (and leave the hour
as adjusted by Date if you previously added an hour) or better: add one hour to
now and set minutes to 0 in that branch, otherwise setMinutes(roundedMinutes);
remove the separate now.setHours(now.getHours() + 1) block so the hour is only
adjusted once.
| const combinedDateTime = this.combineDateTime(startDate, startTime); | ||
| if (!combinedDateTime) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Follow-up: ensure combineDateTime signature updates are applied where used
After making combineDateTime timezone-aware, verify all call sites (validator and onSubmit) pass the timezone.
Run this to confirm only the new signature is used:
🏁 Script executed:
#!/bin/bash
rg -nP 'combineDateTime\s*\(' -C2Length of output: 4065
Update all combineDateTime usages to the new timezone-aware signature
All current calls still use the old two-parameter form. After making combineDateTime timezone-aware, you must:
• Modify the private method signatures in both components to accept a third timezone argument:
- apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-form/meeting-form.component.ts (lines 351–353)
- apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts (lines 349–352)
• Update each call site to pass the timezone:
- meeting-form.component.ts at lines 155 and 471
- meeting-create.component.ts at lines 130 and 321
Example change:
- private combineDateTime(date: Date, time: string): string {
+ private combineDateTime(date: Date, time: string, tz: string): string {
- const combinedDateTime = this.combineDateTime(startDate, startTime);
+ const combinedDateTime = this.combineDateTime(startDate, startTime, this.currentUserTimeZone);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-create/meeting-create.component.ts
around lines 321–322 and in
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-form/meeting-form.component.ts
around lines 351–353, the private combineDateTime method was made timezone-aware
but callers still use the old two-parameter signature; update the private method
signatures in both files to accept a third timezone argument, then update all
call sites to pass the timezone value: meeting-form.component.ts at lines 155
and 471, and meeting-create.component.ts at lines 130 and 321, ensuring the
timezone is forwarded where combineDateTime is invoked.
Summary
Implements the new meeting creation form with updated UX as a multi-step stepper interface. This replaces the previous single-page form with a guided workflow that improves user experience and form completion rates.
Completed Steps:
Key Features:
Test plan
Implements LFXV2-279, LFXV2-280, LFXV2-281, LFXV2-282
🤖 Generated with Claude Code