-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Divyanshu | Test | Convert Tax module into material #14364
base: giddh-2.0
Are you sure you want to change the base?
Conversation
…/Giddh-New-Angular4-App into 86cwf7nuh-tax-stage
Dilpreet | Stage | Prod Bugs
Dilpreet | Stage | Branch Wise UI Changes
WalkthroughThis pull request introduces a series of systematic updates across multiple components in the web-giddh application, focusing on modernizing the UI with Angular Material components, simplifying code structure, and enhancing user experience. The changes span various modules including GST filing, settings, reports, and shared components, with a consistent theme of transitioning from custom implementations to Material Design components and streamlining code organization. Changes
Possibly related PRs
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 22
🔭 Outside diff range comments (13)
apps/web-giddh/src/app/financial-reports/components/filter/filter.component.html (1)
Line range hint
1-450
: Enhance accessibilitySeveral accessibility improvements are recommended:
- Replace
javascript:void(0)
with proper button elements- Add aria-labels to icons and interactive elements
- Ensure proper keyboard navigation support
- Add proper ARIA roles where needed
Example improvements:
- <a href="javascript:void(0)" class="list-item d-flex"> + <button type="button" class="list-item d-flex"> - <span class="icon-refresh"></span> + <span class="icon-refresh" aria-label="Refresh data"></span> - <a class="cp expand-icon" (click)="emitExpand(true)" + <button type="button" class="cp expand-icon" (click)="emitExpand(true)" aria-label="Expand all"apps/web-giddh/src/app/reports/components/payment-report/payment-report.component.html (2)
Line range hint
8-11
: Fix syntax error in the test environment conditionalThe comment indicates test environment code, but there's no clear separation of environments. Consider using Angular's environment files for better environment management.
-<!-- NOTE - This code is only active for Test Environment - Start --> +<ng-container *ngIf="!environment.production"> <button type="button" class="btn btn-sky-blue dropdown-btn-new" [routerLink]="'/pages/vouchers/payment/create'" - *ngIf="(voucherApiVersion === 2) && !isProdMode" + *ngIf="voucherApiVersion === 2" > {{ localeData?.new_voucher }} </button> +</ng-container> -<!-- NOTE - This code is only active for Test Environment - End -->
Line range hint
1-400
: Consider implementing error boundaries for the payment report componentThe template handles loading states but lacks error handling for failed API calls or data loading issues.
Consider adding:
- Error state handling
- User-friendly error messages
- Retry mechanisms for failed operations
This would improve the user experience when encountering errors.
apps/web-giddh/src/app/reports/components/advance-receipt-report/advance-receipt-report.component.html (1)
Line range hint
1-500
: Add explicit error state handlingThe template handles loading and empty states but lacks explicit error state handling.
Consider adding an error state view:
<div class="error-state" *ngIf="errorMessage"> <div class="alert alert-danger"> <i class="icon-error"></i> {{ errorMessage }} <button class="btn btn-link" (click)="retryLoading()"> {{ commonLocaleData?.app_retry }} </button> </div> </div>apps/web-giddh/src/app/models/api-models/Invoice.ts (1)
Line range hint
486-521
: Improve type safety inCustomTemplateResponse
The class uses several
any
types which reduces type safety. Consider using more specific types and grouping design-related properties using an interface.+interface TemplateDesign { + templateColor: string; + tableColor: string; + font: string; + topMargin: number; + leftMargin: number; + rightMargin: number; + bottomMargin: number; + logoSize: string; +} export class CustomTemplateResponse { - public fontSize: any; - public fontMedium?: any; - public fontLarge?: any; - public fontDefault?: any; - public fontSmall?: any; + public fontSize: string; + public fontMedium?: string; + public fontLarge?: string; + public fontDefault?: string; + public fontSmall?: string; // ... other properties + public design: TemplateDesign; }apps/web-giddh/src/app/vat-report/obligations/obligations.component.ts (2)
Line range hint
89-173
: Consider breaking down ngOnInit for better maintainabilityThe ngOnInit method is handling multiple responsibilities including:
- Branch consolidation setup
- Tax details loading
- HMRC authorization
- Obligation list management
- Loading state management
Consider extracting these into separate private methods for better organization:
public ngOnInit(): void { + this.setupBranchConsolidation(); + this.setupTaxAndBranchDetails(); + this.setupObligationSubscriptions(); + this.setupLoadingState(); } +private setupBranchConsolidation(): void { + this.store.pipe( + select(select => select.branchConsolidated), + takeUntil(this.destroyed$) + ).subscribe(response => { + if (response) { + this.isConsolidatedBranch = response.isBranchConsolidated; + } + }); + document.querySelector('body').classList.add('gst-sidebar-open'); + this.getUniversalDatePickerDate(); + this.isCompanyMode = this.generalService.currentOrganizationType === OrganizationType.Company; +}
Line range hint
466-474
: Avoid direct DOM manipulationDirect DOM manipulation using
document.querySelector
should be avoided in Angular applications.Consider using Angular's renderer or CSS classes with
[class.gst-sidebar-open]
binding:-document.querySelector('body').classList.add('gst-sidebar-open'); +// In template +<body [class.gst-sidebar-open]="true"> -document.querySelector('body').classList.remove('gst-sidebar-open'); +// Handle via component property +public isSidebarOpen = false;apps/web-giddh/src/app/gst/filing/tabs/push-to-gstin/components/hsn-summary/hsn-summary.component.ts (1)
Line range hint
1-4
: Add missing imports for constants.The component uses
isElectron
,AppUrl
, andAPP_FOLDER
constants, but their imports are missing.Add the following imports:
import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { ReplaySubject } from 'rxjs'; import { HSNSummary } from '../../../../../../models/api-models/GstReconcile'; import { GstReport } from 'apps/web-giddh/src/app/gst/constants/gst.constant'; +import { isElectron } from 'apps/web-giddh/src/app/utils/electron.util'; +import { AppUrl, APP_FOLDER } from 'apps/web-giddh/src/app/app.constant';apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.html (1)
Line range hint
26-148
: Add accessibility attributes and input validationThe dropdown menus and tabs section needs accessibility improvements and input validation:
- Missing ARIA labels on interactive elements
- No validation feedback for email/SMS inputs
-<textarea class="form-control" #emailList [placeholder]="localeData?.enter_email_addresses"></textarea> +<textarea + class="form-control" + #emailList + [placeholder]="localeData?.enter_email_addresses" + aria-label="Email addresses" + pattern="^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$" + (input)="validateEmails($event.target.value)"> +</textarea> +<mat-error *ngIf="emailError">{{ emailError }}</mat-error>apps/web-giddh/src/app/reports/components/reverse-charge-report-component/reverse-charge-report.component.html (1)
Line range hint
560-576
: Duplicate pagination components
You have both a custom component and a in this template. If your goal is to move fully to Angular Material, consider removing or commenting out the old code to avoid user confusion.Also applies to: 578-585
apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.ts (1)
Responsive behavior needs attention after BreakpointObserver removal
The verification reveals that BreakpointObserver was actively used to handle mobile responsiveness:
- It managed the sidebar state (
asideGstSidebarMenuState
) based on screen width- It tracked mobile screen state (
isMobileScreen
) for viewport width below 767px- These responsive behaviors are now removed without replacement
🔗 Analysis chain
Line range hint
39-46
: Verify responsive behavior after BreakpointObserver removalThe removal of BreakpointObserver might affect the component's responsive behavior. Please ensure that the UI remains functional across different screen sizes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for responsive UI handling in template file rg -l "class.*mobile|class.*sm-|class.*xs-|@media" "apps/web-giddh/src/app/gst/unit-mapping/"Length of output: 93
Script:
#!/bin/bash # Let's check the template file content directly fd "unit-mapping.component.html" apps/web-giddh/src/app/gst/unit-mapping/ --exec cat {} # Also check for any responsive classes or media queries in related CSS/SCSS files fd "unit-mapping.component.(css|scss)" apps/web-giddh/src/app/gst/unit-mapping/ --exec cat {} # Check if BreakpointObserver was actually used in the previous version git show HEAD^ -- apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.ts | grep -A 5 "BreakpointObserver"Length of output: 4293
Script:
#!/bin/bash # Let's check if BreakpointObserver was actually used in any methods or subscriptions git show HEAD^ -- apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.ts | grep -A 20 "ngOnInit\|breakpointObserver" # Also check if there are any responsive layout directives in the template rg "fxLayout|fxFlex|fxHide|fxShow" apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.htmlLength of output: 2290
apps/web-giddh/src/app/company-import-export/component/form/company-import-export-form.html (1)
Line range hint
89-97
: Enhance file upload securityConsider adding additional file type validation beyond just the accept attribute, as it can be bypassed client-side.
<input type="file" id="fileUpload" accept="application/json" + (change)="validateAndHandleFile($event.target.files)" class="d-none" - (change)="fileSelected($event.target.files)" />Add this validation method to the component:
validateAndHandleFile(files: FileList): void { if (!files.length) return; const file = files[0]; if (file.type !== 'application/json') { // Show error return; } if (file.size > 5 * 1024 * 1024) { // 5MB limit // Show error return; } this.fileSelected(files); }apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.component.html (1)
Line range hint
337-563
: Remove commented-out code.Large blocks of commented code should be removed as they add noise and make the file harder to maintain. If this code is needed for reference, it should be preserved in version control history.
Remove all commented-out sections between lines 337-563.
🧹 Nitpick comments (94)
apps/web-giddh/src/app/gst/filing/header/filing-header.component.ts (3)
66-69
: Consider making the naming of both variables consistent for clarity.While "cancelConfirmationDialog" (template ref) and "cancelConfirmationDialogRef" (MatDialogRef) both convey the purpose, naming them using a unified convention (e.g., cancelConfirmationDialogTemplate/cancelConfirmationDialogRef) may enhance clarity.
104-105
: Ensure consistent naming or grouping for aside-related variables.You now have "asideAuthenticationDialog" (TemplateRef) and "asideAuthenticationDialogRef" (MatDialogRef). This is good but double-check that code referencing them—particularly naming and usage in methods—stays consistent across the application.
344-344
: Consider returning the dialog reference for future close or data passing.The openPushToPortalDialog method opens the dialog without storing or returning the reference. If you plan to close the dialog from within another method or retrieve data upon close, consider returning or storing the MatDialogRef.
apps/web-giddh/src/app/gst/filing/header/filing-header.component.html (4)
21-31
: Optimize repeated inline style usageFor better maintainability, consider defining common styles in shared CSS or SCSS files rather than repeating classes or inline styles here.
30-37
: Consider renaming for clarity"openPushToPortalDialog()" suggests pushing data, but it also appears to be triggered by the user to download data. Consider renaming or adding documentation to clarify the function’s purpose.
74-81
: Follow consistent naming conventions for images"bag.svg", "back.svg", and "cart.svg" appear in different sections. Consider centralizing or standardizing naming patterns for icons to promote consistent referencing and reduce confusion about their usage.
91-127
: Enhance button accessibilityConsider adding aria-label text or role attributes to the Material buttons for better accessibility support, especially for assistive technologies.
apps/web-giddh/src/app/settings/taxes/setting.taxes.component.html (5)
8-9
: Use a consistent approach for keyboard shortcuts.
The "alt+c" keyboard shortcut toggles the tax aside pane, but the code also uses a separate click handler. If there are other keyboard shortcuts used in the app, consider a centralized approach (e.g., a dedicated directive or service) for improved maintainability and discoverability.
13-22
: Remove stale commented-out code to reduce clutter.
The old block of commented-out button lines no longer appear to serve a purpose now that a new “Create new tax” button is in place. Commented-out code can confuse future maintainers and should be removed if not needed.
36-39
: Consider lazy loading or pagination for large data sets.
When a large number of tax entries exist, rendering potentially hundreds of rows in the Material table could negatively affect performance. Implementing built-in Material table features (like pagination) or lazy loading would enhance user experience.
107-135
: Avoid repeated inline button classes and consider a shared style or directive.
Multiple Material buttons share similar classes and inline usage. Centralizing repeated class usage into a single style or directive can simplify code maintenance.
141-357
: Remove or refactor large blocks of commented-out code if no longer relevant.
This old table markup significantly increases the file size and can cause confusion. If the code is fully replaced by Material tables, it’s likely safe to remove and rely on source control for history.apps/web-giddh/src/app/financial-reports/components/filter/filter.component.html (1)
Line range hint
1-450
: Consider broader Material Design adoptionSince this PR aims to convert components to Material Design, consider these improvements:
- Replace Bootstrap modals with Material Dialog
- Convert form controls to Material Form Fields
- Use Material Date Picker instead of custom implementation
- Implement Material Icons instead of custom icon classes
- Utilize Material's expansion panel for expand/collapse functionality
This would provide:
- Consistent UI/UX across the application
- Better accessibility
- Built-in responsive design
- Reduced bundle size by removing Bootstrap dependencies
Would you like me to provide specific examples of how to implement these Material components?
apps/web-giddh/src/app/reports/components/payment-report/payment-report.component.html (2)
Line range hint
1-24
: Consider improving accessibility for the hamburger menu and filter buttonsAdd ARIA labels and roles to improve screen reader support:
-<hamburger-menu [pageHeading]="localeData?.page_heading"></hamburger-menu> +<hamburger-menu [pageHeading]="localeData?.page_heading" role="navigation" aria-label="Main menu"></hamburger-menu> -<button type="button" class="btn btn-sky-blue dropdown-btn-new" +<button type="button" class="btn btn-sky-blue dropdown-btn-new" aria-label="Create new voucher"
Line range hint
32-38
: Consider implementing virtual scrolling for large datasetsThe table implementation could benefit from virtual scrolling to improve performance when dealing with large datasets. Consider using Angular Material's virtual scroll capability.
Consider implementing:
cdk-virtual-scroll-viewport
for the table body- Height constraints to enable efficient rendering
- Buffer size optimization based on your typical dataset size
This would improve performance especially when loading hundreds of payment records.
Also applies to: 171-187
apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.ts (3)
30-30
: Fix spacing after @input() decoratorThere should be a space between the @input() decorator and the property declaration.
-@Input()public currentVoucherFilter: string; +@Input() public currentVoucherFilter: string;
289-292
: Improve error handling in downloadInvoice methodThe error handling shows the same generic message for both error response and error callback. Consider:
- Consolidating the error handling
- Providing more specific error messages based on the error type
- this._toasty.showSnackBar('error', this.commonLocaleData?.app_something_went_wrong); - } - }, (error => { - this._toasty.showSnackBar('error', this.commonLocaleData?.app_something_went_wrong); + this._toasty.showSnackBar('error', response.message || this.commonLocaleData?.app_something_went_wrong); + } + }, (error => { + this._toasty.showSnackBar('error', error.message || this.commonLocaleData?.app_something_went_wrong);
Line range hint
1-307
: Consider architectural improvements for better maintainabilityThe component handles multiple responsibilities and contains complex logic. Consider these improvements:
Split into smaller components:
- InvoiceDownloadComponent
- InvoiceEmailComponent
- InvoiceSmsComponent
Extract business logic into services:
- InvoiceDownloadService
- InvoiceCommunicationService
Standardize error handling patterns across all methods
This would improve maintainability, testability, and make the code more modular.
apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.module.ts (1)
24-27
: Consider moving Ladda configuration to a separate config file.The inline Ladda configuration could be moved to a separate configuration file to improve maintainability and reusability across the application.
apps/web-giddh/src/app/gst/modals/push-to-portal/push-to-portal.component.ts (1)
16-17
: Enhance the property documentation.Consider adding more context to the JSDoc comment, such as the purpose and usage of this link.
- /** Holds GST utility download link */ + /** URL to the GST utility download page on the official GST portal. + * Used to provide users with direct access to download GST utilities and tools. */apps/web-giddh/src/app/reports/components/advance-receipt-report/advance-receipt-report.component.html (4)
49-49
: Consider using object syntax for ngClassThe ternary operator in the class binding can be made more readable using ngClass's object syntax.
-[ngClass]="option?.consolidatedBranch ? 'icon-company-profile' : 'icon-branch-icon'" +[ngClass]="{ + 'icon-company-profile': option?.consolidatedBranch, + 'icon-branch-icon': !option?.consolidatedBranch +}"
Line range hint
1-500
: Enhance table accessibilityThe table structure would benefit from additional accessibility attributes to improve screen reader support:
- Add
aria-sort
attributes to sortable columns- Add
aria-label
to the table element describing its purpose- Consider adding
scope="col"
to table headersExample implementation:
-<table class="table basic giddh-table on-mobile-view" +<table class="table basic giddh-table on-mobile-view" + aria-label="Advanced Receipt Report"-<th>{{ commonLocaleData?.app_date }} +<th scope="col" [attr.aria-sort]="searchQueryParams.sortBy === 'voucherDate' ? searchQueryParams.sort : 'none'"> + {{ commonLocaleData?.app_date }}
Line range hint
1-500
: Optimize template performanceSeveral performance improvements could be made:
- Use
trackBy
function with*ngFor
directives to optimize rendering:-*ngFor="let receipt of allReceipts; let i = index" +*ngFor="let receipt of allReceipts; let i = index; trackBy: trackByFn"
- Move complex calculations to the component:
-*ngIf="!allReceipts?.length && !searchQueryParams?.receiptNumber" +*ngIf="shouldShowEmptyState"
- Consider using
OnPush
change detection strategy in the component to reduce change detection cycles.
Line range hint
1-500
: Enhance user experienceConsider the following UX improvements:
- Add loading skeletons instead of a simple spinner:
<tr *ngFor="let i of [1,2,3]" class="skeleton-row" [style.opacity]="isLoading ? 1 : 0"> <td *ngFor="let j of [1,2,3,4,5,6,7,8,9]"> <div class="skeleton-loader"></div> </td> </tr>
- Add keyboard navigation support for the table rows
- Consider adding a "Clear all filters" button when multiple filters are active
- Add tooltips explaining the sorting functionality
apps/web-giddh/src/app/models/api-models/Invoice.ts (1)
Line range hint
471-484
: Consider consolidating identical interfaces
CreatedBy
andUpdatedBy
interfaces have identical structure. Consider creating a single interface to improve maintainability.-export interface CreatedBy { - name: string; - email: string; - uniqueName: string; - mobileNo: string; -} - -export interface UpdatedBy { +export interface UserInfo { name: string; email: string; uniqueName: string; mobileNo: string; }Then use this interface:
export interface CustomTemplateResponse { createdBy: UserInfo; updatedBy: UserInfo; // ... other properties }apps/web-giddh/src/app/reports/components/reverse-charge-report-component/reverse-charge-report.component.ts (2)
254-260
: Remove commented codeSince the code is version controlled, we should remove the commented-out code block instead of leaving it in the codebase.
- // public pageChanged(event: any): void { - // if (this.reverseChargeReportGetRequest.page != event.page) { - // this.reverseChargeReportResults.results = []; - // this.reverseChargeReportGetRequest.page = event.page; - // this.getReverseChargeReport(false); - // } - // }
497-508
: Simplify handleClickOutside logicThe current implementation has nested conditions that can be simplified for better readability:
public handleClickOutside(event: any, element: any, searchedFieldName: string): void { - if (searchedFieldName === "name") { - if (this.searchedName?.value) { - return; - } - if (this.generalService.childOf(event.target, element)) { - return; - } else { - this.showNameSearch = false; - } - } + if (searchedFieldName !== "name") { + return; + } + + if (this.searchedName?.value || this.generalService.childOf(event.target, element)) { + return; + } + + this.showNameSearch = false; }apps/web-giddh/src/app/vat-report/obligations/obligations.component.ts (3)
Line range hint
1-21
: Good architectural improvements!The transition to Angular Material dialogs and the use of ComponentStore for state management shows good architectural decisions. The removal of mobile-specific dependencies also helps in streamlining the codebase.
Consider documenting the state management patterns in the component for better maintainability.
Line range hint
175-191
: Optimize RxJS usage in getCurrentCompanyBranchTaxNumberThe current implementation uses nested subscriptions which could be simplified using RxJS operators.
Consider refactoring to use RxJS operators:
private getCurrentCompanyBranchTaxNumber(): void { - this.componentStore.currentCompanyBranches$.pipe(takeUntil(this.destroyed$)).subscribe(response => { - if (response) { - const currentBranchUniqueName = this.generalService.currentBranchUniqueName; - let currentBranch = response.find(branch => branch?.uniqueName === currentBranchUniqueName); - this.hasTaxNumber = currentBranch?.addresses?.filter(address => address?.taxNumber?.length > 0)?.length > 0; - if (this.hasTaxNumber) { - this.loadTaxDetails(); - } - } - }); + this.componentStore.currentCompanyBranches$.pipe( + takeUntil(this.destroyed$), + map(response => { + if (!response) return false; + const currentBranch = response.find( + branch => branch?.uniqueName === this.generalService.currentBranchUniqueName + ); + return currentBranch?.addresses?.some(address => address?.taxNumber?.length > 0) ?? false; + }), + tap(hasTaxNumber => { + this.hasTaxNumber = hasTaxNumber; + if (hasTaxNumber) { + this.loadTaxDetails(); + } + }) + ).subscribe(); }
Line range hint
289-339
: Consider standardizing UI framework usageThe component mixes Angular Material (for dialogs) and ngx-bootstrap (for datepicker) which could lead to inconsistent UI/UX.
Consider migrating all components to Angular Material for consistency:
- Replace ngx-bootstrap datepicker with MatDatepicker
- Use Material's overlay system instead of bootstrap modals
- Standardize styling to follow Material Design guidelines
apps/web-giddh/src/app/gst/filing/tabs/push-to-gstin/components/hsn-summary/hsn-summary.component.ts (1)
26-39
: Consider using camelCase for column identifiers.The implementation of displayedColumns is functionally correct, but consider using camelCase for column identifiers to better align with Angular Material's conventions (e.g., 'hsnSc' instead of 'hsn_sc'). This would improve consistency with Angular's style guide.
public displayedColumns: string[] = [ - 'hsn_sc', + 'hsnSc', 'desc', 'qty', 'uqc', 'txval', 'rt', 'iamt', 'camt', 'samt', 'csamt', 'val' ];apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.html (4)
28-28
: Consider using Angular's form features on this "TAX PRO || VYANA" form.Currently, the form is declared with a plain
tag. It can be beneficial to leverage Angular’s reactive or template-driven form approach (e.g., #taxProForm="ngForm") for improved validation, testability, and maintainability.
70-72
: Avoid using "href='javascript:;'".Using href="javascript:;" is a practice that can lead to unexpected behavior and is not easily identifiable by assistive technologies. Consider using a button element or (click) handler on a instead to maintain accessibility and clarity.
Here’s a snippet for turning this into a proper button:
-<a class="btn btn-link" (click)="resendOtp()" href="javascript:;">{{ localeData?.aside_menu?.resend_otp }}</a> +<button class="btn btn-link" type="button" (click)="resendOtp()"> + {{ localeData?.aside_menu?.resend_otp }} +</button>
179-316
: Check consistency of Material button styles and color usage.You are mixing mat-stroked-button in places typically reserved for primary actions. For a more standard Material Design approach, consider using mat-raised-button with color="primary" for principal actions (like “Proceed” or “Submit”) and mat-stroked-button or mat-flat-button for secondary actions. This aids consistency and user experience.
245-251
: Consider abstracting the date range display logic.Currently, you display the filing period with a straightforward “from - to” approach. If multiple components or parts of the application require date range formatting, consider creating a shared method or pipe to format these ranges consistently. This fosters reusability and keeps the template uncluttered.
apps/web-giddh/src/app/theme/form-fields/input-field/input-field.component.html (1)
7-10
: Enhance accessibility for the label iconWhile the icon addition looks good, consider improving accessibility for screen readers.
Apply this diff to add ARIA attributes:
<mat-label> {{ label ? label : placeholder }} - <i *ngIf="showLabelIcon" [class]="labelIconClass" [matTooltip]="labelIconTooltip" matTooltipPosition="above"></i> + <i *ngIf="showLabelIcon" + [class]="labelIconClass" + [matTooltip]="labelIconTooltip" + matTooltipPosition="above" + role="img" + [attr.aria-label]="labelIconTooltip"></i> </mat-label>apps/web-giddh/src/app/theme/form-fields/input-field/input-field.component.ts (2)
85-90
: Add JSDoc documentation for new input propertiesThe new properties need proper documentation to improve maintainability and developer experience.
Apply this diff to add JSDoc comments:
+ /** Flag to control the visibility of the label icon */ @Input() public showLabelIcon: boolean = false; + /** CSS class(es) to be applied to the label icon. Expected format: "fa fa-icon-name" */ @Input() public labelIconClass: string = "fa fa-info-circle"; + /** Tooltip text to be displayed when hovering over the label icon. Should not be null when showLabelIcon is true */ @Input() public labelIconTooltip: string = null;Also consider adding runtime validation:
@Input() public set labelIconTooltip(value: string) { this._labelIconTooltip = value; if (this.showLabelIcon && !value) { console.warn('labelIconTooltip should not be empty when showLabelIcon is true'); } } public get labelIconTooltip(): string { return this._labelIconTooltip; } private _labelIconTooltip: string = null;
85-90
: Consider adding icon color customizationSince this component is part of the Material Design migration, consider adding support for Material theme colors.
Add a new input property for icon color:
+ /** Material theme color to be applied to the icon. Default: primary */ + @Input() public labelIconColor: 'primary' | 'accent' | 'warn' = 'primary';Then update the template:
- <i *ngIf="showLabelIcon" [class]="labelIconClass" [matTooltip]="labelIconTooltip" matTooltipPosition="above"></i> + <i *ngIf="showLabelIcon" + [class]="labelIconClass" + [matTooltip]="labelIconTooltip" + matTooltipPosition="above" + [color]="labelIconColor"></i>apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.ts (2)
216-221
: Enhance method documentationWhile adding JSDoc and return type is good, the documentation could be more descriptive by explaining the method's purpose and the event it emits.
Consider updating the documentation:
/** - * Toggle cancel model + * Toggles the cancel confirmation modal and emits an event to notify parent component * + * @emits cancelConfirmationEvent - Emits true when cancel is requested * @memberof GstAsideMenuComponent */
Line range hint
1-275
: Consider architectural improvements for better maintainabilityThe component has grown complex with multiple responsibilities. Consider the following improvements:
Break down into smaller sub-components:
- Authentication component for OTP flow
- Provider selection component
- GST filing component
Improve state management:
- Use async pipe in template instead of subscribing in component
- Consider using NgRx component store for local state
- Add proper typing for form objects
Enhance error handling:
- Add error states for API failures
- Implement proper error messages for each failure scenario
Would you like help creating a proposal for breaking down this component into smaller, more manageable pieces?
apps/web-giddh/src/app/settings/address-settings/address-settings.component.html (1)
Line range hint
1-400
: Consider breaking down the template into smaller componentsThe template is quite large and contains repeated patterns. Consider:
- Extract the company address table into a separate component
- Extract the branch address list into a separate component
- Extract the search template into a reusable component
- Move modal templates to separate files
This would improve:
- Code maintainability
- Reusability
- Testing capabilities
- Performance through better change detection
apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.html (1)
7-23
: Consider using Material Design buttons for consistencyThe dialog header uses Material Design classes but the buttons still use Bootstrap classes (
btn-md
,btn-info
). For consistency with the Material Design migration, consider using Material buttons.-<button type="button" class="btn btn-md btn-info mr-r1" *ngIf="showEditButton" (click)="editVoucher()"> +<button mat-button color="primary" *ngIf="showEditButton" (click)="editVoucher()"> {{ commonLocaleData?.app_edit }} </button> -<button type="button" class="btn btn-md btn-default mr-r1" (click)="showMore = !showMore; showEsign = false"> +<button mat-button (click)="showMore = !showMore; showEsign = false"> {{ commonLocaleData?.app_more }} <span class="caret"></span> </button>apps/web-giddh/src/app/reports/components/reverse-charge-report-component/reverse-charge-report.component.html (2)
21-21
: Extract inline style to a stylesheet
Inline styles (e.g. style="left: 0;") can make the code harder to maintain. Consider moving these styles to a dedicated CSS/SCSS class for better maintainability.
Line range hint
298-559
: Consider removing large blocks of commented-out legacy code
The commented-out table markup may cause confusion and clutter. If it's no longer needed, remove it to streamline the template.apps/web-giddh/src/app/gst/modals/push-to-portal/push-to-portal.component.html (2)
1-4
: Enhance accessibility and Material Design consistencyConsider these improvements for better accessibility and Material Design alignment:
-<div class="dialog-header"> +<div class="dialog-header" role="dialog" aria-labelledby="modal-title"> - <h3 class="modal-title">{{ localeData?.filing?.push_to_portal }}</h3> + <h3 class="modal-title" id="modal-title">{{ localeData?.filing?.push_to_portal }}</h3> - <i mat-dialog-close aria-label="close icon" class="icon-cross"></i> + <button mat-icon-button mat-dialog-close aria-label="Close dialog"> + <mat-icon>close</mat-icon> + </button> </div>
5-52
: Consider using Angular Material's flex layout for better responsivenessThe current grid layout could be improved using Material's flex layout system for better control over responsive behavior:
-<div class="container"> - <div class="row main-container"> - <div class="col left-side"> +<div class="container" fxLayout="row" fxLayout.lt-md="column" fxLayoutGap="20px"> + <div fxFlex="50" fxFlex.lt-md="100" class="left-side">This would provide:
- Better control over column widths
- Smoother responsive behavior
- Consistent spacing between columns
apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.ts (6)
40-43
: Visibility and usage of ViewChild references.
You've introduced vehicleDialog and cancelDialog to open Angular Material dialogs. Ensure that these references (and their associated templates) are correctly used and code from the older ngx-bootstrap ModalService references is fully removed where no longer needed.
164-165
: Constructor injection for new services.
The addition of GstReconcileService and MatDialog is a solid approach. Make sure that you handle disposal of any possible subscriptions from these services if they internally manage Observables.
205-205
: Initialize transDocDate with current date.
This is straightforward. If you need local time zone or advanced date resolution (e.g., UTC vs local), keep in mind dayjs default behavior.
451-461
: Method openAddVehicleDialog uses Angular Material dialog.
A straightforward approach. Ensure you handle the dialog’s closing event if additional actions are needed (e.g., refreshing data or clearing form states).Would you like guidance on triggering a refresh or cleanup handler upon dialog closure?
477-489
: Refactored cancelEwayBill to accept raw form values.
Switching from an NgForm reference to a raw object is often more flexible. Confirm that the validations within the form remain intact.
756-767
: Handling tab change logic.
You’re effectively capturing the index and textLabel from the selected tab. Consider verifying that each tab’s content is well-defined or loaded only when active if performance is a concern.apps/web-giddh/src/app/gst/filing/tabs/push-to-gstin/components/hsn-summary/hsn-summary.component.html (1)
14-21
: Consider using a more robust truncation or ellipsis for long descriptions
Your current approach uses matTooltip to show the full description. If the field can become very large, you might consider adding character-limits or line-clamp for better readability in the cell, relying on the tooltip for the complete content.apps/web-giddh/src/app/gst/filing/tabs/overview/summary/summary.component.html (1)
13-14
: Mat-table usage
Swapping a custom table for mat-table is a good move for consistency. If the data set can be large, consider using trackBy functions and/or pagination for performance.apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.html (1)
25-45
: Dropdown logic
Lines 25-45 handle two different sets of entity types (gstr1entityType vs gstr2entityType). If additional variations are introduced (e.g., gstr3), consider dynamically populating these arrays instead of manually branching.apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.ts (8)
2-2
: **Use explicit scoping on import statements **
While this import is valid, consider confirming that all imported symbols here are necessary. If some are unused, removing them can slightly optimize your bundle size and clarify your dependencies.
19-20
: **Angular best practice: centralize constants **
Importing from "apps/web-giddh/src/app/app.constant" is acceptable. However, storing shared constants like PAGE_SIZE_OPTIONS and reusing them in multiple modules or services helps keep your code consistent and easier to maintain.
45-46
: **Meaningful naming for ease of understanding **
“downloadOrSendMailDialog” is a descriptive template reference name, but consider using a shorter, consistent prefix if you plan on introducing multiple template-driven dialogs.
79-95
: **Consistent naming and logic alignment **
The displayedColumns array thoroughly lists table columns. Confirm that any subsequent logic (sorting, filtering, UI toggling) references these columns consistently. If there are future merges or toggling of columns, consider retrieving these from a config or enumeration.
103-106
: **Constructor injection ordering **
Consider grouping DI parameters logically (e.g., NxNGRX store, services, then local project imports) to keep them consistent and easier to read. This is purely stylistic but can improve scanability.Also applies to: 111-112
206-230
: **Flow clarity in “onSelectInvoice” **
This logic checks voucher version and dispatches a store action accordingly. Consider extracting the repeated logic into a helper method or service if it’s reused frequently, especially with multiple voucher types.🧰 Tools
🪛 Biome (1.9.4)
[error] 215-215: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
349-349
: **Error handling consistency **
Using showSnackBar is consistent with the rest of the file. Confirm that the entire codebase also leverages this approach instead of duplicating toasts with different service calls.
384-396
: **Encapsulate Material Dialog config **
Consider specifying the dialog’s config in a unified place or a configuration object so that multiple dialogs can share consistent height/width defaults, especially if you maintain multiple dialogs across the application.apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.html (5)
19-48
: **Responsive design and dynamic forms **
The combined usage of “dropdown-field” and a custom datepicker is good, but ensure consistent placeholder text and that the selected entity matches the correct data. Check for edge cases when no branches exist or only a single branch is present.
54-60
: **Conditional rendering (mat-tab-group) **
You are hiding the mat-tab-group if the array is empty or the request is in progress. If you plan to handle multiple states (like partial data load or error states), consider providing a fallback UI for those conditions.
Would you like help establishing unambiguous fallback displays or skeleton screens?
93-101
: **Customer details clarity **
Both “customerName” and “customerGstin” columns appear side by side. Ensuring uniform display logic (e.g., uppercase GSTIN, or limiting text if it’s too long) might improve readability.
114-117
: **Date format consistency **
You’re directly rendering “{{ item?.ewayBillDate }}”. Confirm that date formats match the rest of the application (e.g., a global date pipe or a user-locale-based date string).
119-133
: **Amount formatting **
Using “” is great. If your code repeats this logic, consider centralizing the currency code logic (e.g., currency symbol vs. code) in a single place so it’s consistent across various tables.apps/web-giddh/src/app/gst/gst.component.html (4)
Line range hint
122-156
: **Transaction counts check **
Your “gstr1TransactionCounts” check triggers the “view_summary” button. For clarity, ensure that potential undefined or null states of “gstr1TransactionCounts” yield the correct fallback UI.
265-280
: **Pull-right vs. material alignment **
This button uses “pull-right” while others use “mr-t1” or flex justification. For consistent styling, consider switching all alignment to Angular Material’s design parameters or a single utility class.
327-333
: **Placeholder icons **
Using SVG placeholders for “create transactions” is good. If you intend future expansions, consider a consistent icon library for theming.
351-372
: **Empty states for GSTR2 **
Similar approach to GSTR1. If you’d like to unify these “create transactions” placeholders or introduce a skeleton loading state, I can help with a code snippet or separate GitHub issue.Also applies to: 374-396
apps/web-giddh/src/app/gst/gstR3/gstR3.component.ts (5)
15-15
: **Formatting consistency **
The newly added GIDDH_DATE_FORMAT_MONTH_YEAR is well-named. Double-check that references throughout the code are consistent with the import and do not inadvertently mix different date formats.
126-129
: **Setting default month in ngOnInit **
This logic assigns selectedMonth from currentPeriod.from. If currentPeriod is missing or incorrectly formatted, you might get an invalid date. Consider a fallback or additional checks for user-specified periods outside the expected year range.
246-264
: **setGstrUserTableData method **
Two items are being pushed with number and label. If you plan on expanding user data (e.g., more lines or dynamic fields), a more scalable approach might parse the object keys automatically. Would you like help with that?
710-736
: **Month selection for GSTR3 **
In setMonthAndYear and dateSelectedEvent, you handle partial transformations for the date. If expanded or reused in other flows, consider centralizing these transformations in a date utility service.
737-750
: **Translation re-check **
You re-run set tables after translation completes. If future expansions require dynamic placeholders or user-driven locale changes, a watch or subscription might handle it more elegantly. Let me know if you’d like a sample code snippet.apps/web-giddh/src/app/contact/contact.component.ts (2)
251-251
: Consider providing stricter typing for bulkEmailSuccess$.
It is currently defined as “any”. For better maintainability, define a more explicit type, e.g., Observable or Observable, depending on the actual data structure returned.
437-449
: Consider modularizing the logic for customizing columns.
The block that toggles the “action” column for vendor tab might benefit from a small helper method to keep the component cleaner. This also reduces duplication if similar logic is used elsewhere.apps/web-giddh/src/app/settings/taxes/confirmation/confirmation.model.component.html (2)
2-5
: Add ARIA label for accessibility.
This header includes a visually hidden close button. Consider adding an aria-label to the close button for a better screen reader experience.
23-23
: Remove unused commented sections if unneeded.
There is a large commented block of old modal code. Delete it if you’ve fully replaced it with the Material approach.apps/web-giddh/src/app/new-inventory/component/inventory-transaction-list/inventory-transaction-list.component.scss (1)
Line range hint
221-227
: Document the responsive design breakpointThe addition of min-width for total-details-box at 1200px breakpoint looks good for preventing content from becoming too cramped. However:
Consider adding a comment explaining:
- Why 1200px was chosen as the breakpoint
- Why 188px was chosen as the min-width
- What layout issues this fixes
+/* Prevents total details from becoming too cramped on medium-sized screens */ @media screen and (max-width: 1200px) { .inventory-transaction-list{ .total-details-box { min-width: 188px; } } }
apps/web-giddh/src/app/gst/filing-status/filing-status.component.html (1)
64-75
: Remove duplicate showFirstLastButtons propertyThe Material paginator implementation looks good with proper configuration. However, there's a duplicate property.
Remove the duplicate showFirstLastButtons property:
<mat-paginator class="mt-3" [showFirstLastButtons]="true" [pageSizeOptions]="pageSizeOptions" [pageIndex]="pageIndex" (page)="pageChanged($event)" [length]="pagination.totalItems" [pageSize]="pagination.count" - [showFirstLastButtons]="true" [hidePageSize]="false" ></mat-paginator>
apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.ts (1)
34-37
: Consider adding type definitions for locale propertiesThe new locale properties should have proper TypeScript interfaces to ensure type safety and better maintainability.
-public localeData: any = {}; -public commonLocaleData: any = {}; +interface LocaleData { + [key: string]: string; } +public localeData: LocaleData = {}; +public commonLocaleData: LocaleData = {};apps/web-giddh/src/app/gst/filing/tabs/overview/summary/summary.component.ts (1)
93-118
: Consider enhancing type safety in the transformation logic.While the transformation logic is sound, we can improve type safety and readability.
Consider this enhanced version:
private transformedSummaryData(data: GstOverViewResult): GstOverViewResult { const results = cloneDeep(data); if (results?.summary?.length) { - const transformedSummary = []; + const transformedSummary: GstOverViewSummary[] = []; results.summary.forEach((item, index) => { transformedSummary.push(item); if (Array.isArray(item.transactions)) { transformedSummary.push( - ...item.transactions.map(transaction => ({ + ...item.transactions.map((transaction): GstOverViewSummary => ({ ...transaction, parentIndex: index })) ); } }); results.summary = transformedSummary; } return results; }apps/web-giddh/src/app/gst/filing-status/filing-status.component.ts (1)
159-162
: Add validation for negative page indicesThe pageChanged method should validate for negative page indices to prevent potential issues with the API call.
if (typeof event?.pageIndex === 'number') { + if (event.pageIndex < 0) { + return; + } this.pageIndex = event.pageIndex; this.pagination.page = event.pageIndex + 1; this.pagination.count = event.pageSize; this.getGstrReferences(); }apps/web-giddh/src/app/gst/filing/tabs/reconcilation/reconcilation.component.ts (2)
94-119
: Simplify tab action mappingThe switch statement can be simplified using a mapping object for better maintainability.
+ private readonly TAB_ACTIONS = { + [this.localeData?.filing?.missing_in_gstn]: 'notfoundonportal', + [this.localeData?.filing?.missing_in_giddh]: 'notfoundongiddh', + [this.localeData?.filing?.partially_matched]: 'partiallymatched', + [this.localeData?.filing?.matched]: 'matched' + }; public reconcileTabChanged(event: MatTabChangeEvent) { if (typeof event?.index === 'number') { this.activeTabIndex = event.index; this.selectedTab = event.tab.textLabel; - let action = ''; - switch (event.tab.textLabel) { - case this.localeData?.filing?.missing_in_gstn: - action = 'notfoundonportal'; - break; - case this.localeData?.filing?.missing_in_giddh: - action = 'notfoundongiddh'; - break; - case this.localeData?.filing?.partially_matched: - action = 'partiallymatched'; - break; - case this.localeData?.filing?.matched: - action = 'matched'; - break; - default: - action = 'notfoundonportal'; - } + const action = this.TAB_ACTIONS[event.tab.textLabel] || 'notfoundonportal'; this.reconcileActiveTab = GstReconcileActionsEnum[action]; const pageInfo = this.getPageInfo(); this.fireGstReconcileRequest(this.reconcileActiveTab, pageInfo.pageNumber, false, pageInfo.count); } }
158-184
: Use optional chaining for safer property accessAs suggested by static analysis, use optional chaining for safer property access in the getPageInfo method.
- if (data && data.data) { + if (data?.data) { page.pageNumber = data.data.page; page.count = data.data.count; }🧰 Tools
🪛 Biome (1.9.4)
[error] 158-158: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 166-166: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 174-174: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 182-182: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web-giddh/src/app/invoice/invoice.module.ts (1)
78-85
: LGTM! Systematic transition to Angular MaterialThe addition of Material modules is well-structured and follows a clear pattern of modernizing the UI components.
Consider the following recommendations for a smooth Material transition:
- Ensure consistent usage of Material components across templates
- Plan for incremental migration of existing Bootstrap components
- Consider creating a shared MaterialModule to centralize Material imports
Also applies to: 168-173, 178-179
apps/web-giddh/src/app/settings/taxes/setting.taxes.component.ts (1)
38-40
: Update ViewChild decorator usageThe commented-out code and new ViewChild implementations show a transition from Bootstrap Modal to Material Dialog.
Consider removing the commented-out code since it's no longer needed:
- // @ViewChild('taxConfirmationModel', { static: true }) public taxConfirmationModel: ModalDirective;
apps/web-giddh/src/app/gst/filing/filing.component.ts (1)
291-296
: Consider adding a default return value.While the null check is good, the method should return a default value when localeData is null.
public getLoadingGstText(selectedGst: any): string { - if (this.localeData) { - let text = this.localeData?.filing?.loading_gst_data; - text = text?.replace("[SELECTED_GST]", selectedGst); - return text; - } + const text = this.localeData?.filing?.loading_gst_data; + return text ? text.replace("[SELECTED_GST]", selectedGst) : `Loading GST data for ${selectedGst}`; }apps/web-giddh/src/app/settings/settings.component.ts (2)
142-142
: Fix indentation.The line has inconsistent indentation with surrounding code.
- } + }
317-320
: Simplify toggleGstPane method.The method can be simplified to a single line since it only performs a single condition check.
- public toggleGstPane(): void { - if (this.asideGstSidebarMenuState === 'in') { - this.asideGstSidebarMenuState = "out"; - } - } + public toggleGstPane(): void { + this.asideGstSidebarMenuState = this.asideGstSidebarMenuState === 'in' ? 'out' : 'in'; + }apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.component.html (1)
11-15
: Remove redundant comments.These comments are no longer needed as the code has been migrated to Material Design.
- <!-- <div class="aside-pane" role="dialog" aria-labelledby="modal-title" [keyboardShortcut]="'esc'" (onShortcutPress)="closeEvent.emit()"> --> - <!-- <div class="form-header"> - <h2 class="aside-title" tabindex="0">{{ tax && tax?.uniqueName ? localeData?.update_tax : localeData?.create_new_tax }}</h2> - </div> --> - <!-- <button id="closedialog" (click)="closeEvent.emit()" aria-label="cross"><i class="icon-cross"></i></button> -->
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
apps/web-giddh/src/app/app.constant.ts
(1 hunks)apps/web-giddh/src/app/company-import-export/component/form/company-import-export-form.html
(1 hunks)apps/web-giddh/src/app/contact/aging-report/aging-report.component.html
(2 hunks)apps/web-giddh/src/app/contact/contact.component.ts
(2 hunks)apps/web-giddh/src/app/financial-reports/components/filter/filter.component.html
(1 hunks)apps/web-giddh/src/app/gst/filing-status/filing-status.component.html
(1 hunks)apps/web-giddh/src/app/gst/filing-status/filing-status.component.ts
(4 hunks)apps/web-giddh/src/app/gst/filing/filing.component.html
(2 hunks)apps/web-giddh/src/app/gst/filing/filing.component.ts
(5 hunks)apps/web-giddh/src/app/gst/filing/header/filing-header.component.html
(2 hunks)apps/web-giddh/src/app/gst/filing/header/filing-header.component.ts
(14 hunks)apps/web-giddh/src/app/gst/filing/tabs/overview/summary/summary.component.html
(1 hunks)apps/web-giddh/src/app/gst/filing/tabs/overview/summary/summary.component.scss
(0 hunks)apps/web-giddh/src/app/gst/filing/tabs/overview/summary/summary.component.ts
(4 hunks)apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.html
(2 hunks)apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.scss
(1 hunks)apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.ts
(9 hunks)apps/web-giddh/src/app/gst/filing/tabs/push-to-gstin/components/hsn-summary/hsn-summary.component.html
(1 hunks)apps/web-giddh/src/app/gst/filing/tabs/push-to-gstin/components/hsn-summary/hsn-summary.component.ts
(1 hunks)apps/web-giddh/src/app/gst/filing/tabs/push-to-gstin/push-to-gstin.component.html
(1 hunks)apps/web-giddh/src/app/gst/filing/tabs/reconcilation/reconcilation.component.html
(6 hunks)apps/web-giddh/src/app/gst/filing/tabs/reconcilation/reconcilation.component.scss
(1 hunks)apps/web-giddh/src/app/gst/filing/tabs/reconcilation/reconcilation.component.ts
(4 hunks)apps/web-giddh/src/app/gst/gst-setting/gst-setting.component.html
(0 hunks)apps/web-giddh/src/app/gst/gst.component.html
(7 hunks)apps/web-giddh/src/app/gst/gst.component.scss
(1 hunks)apps/web-giddh/src/app/gst/gst.component.ts
(8 hunks)apps/web-giddh/src/app/gst/gst.module.ts
(2 hunks)apps/web-giddh/src/app/gst/gstR3/gstR3.component.scss
(1 hunks)apps/web-giddh/src/app/gst/gstR3/gstR3.component.ts
(7 hunks)apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.html
(1 hunks)apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.scss
(1 hunks)apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.ts
(2 hunks)apps/web-giddh/src/app/gst/modals/push-to-portal/push-to-portal.component.html
(1 hunks)apps/web-giddh/src/app/gst/modals/push-to-portal/push-to-portal.component.ts
(2 hunks)apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.html
(2 hunks)apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.scss
(1 hunks)apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.ts
(2 hunks)apps/web-giddh/src/app/home/components/cr-dr-list/cr-dr-list.component.ts
(1 hunks)apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.html
(4 hunks)apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.scss
(0 hunks)apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.ts
(12 hunks)apps/web-giddh/src/app/invoice/invoice.module.ts
(2 hunks)apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.html
(2 hunks)apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.scss
(0 hunks)apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.ts
(3 hunks)apps/web-giddh/src/app/models/api-models/GstReconcile.ts
(1 hunks)apps/web-giddh/src/app/models/api-models/Invoice.ts
(1 hunks)apps/web-giddh/src/app/new-inventory/component/adjust-inventory-list/adjust-inventory-list.component.ts
(1 hunks)apps/web-giddh/src/app/new-inventory/component/inventory-transaction-list/inventory-transaction-list.component.scss
(1 hunks)apps/web-giddh/src/app/reports/components/advance-receipt-report/advance-receipt-report.component.html
(1 hunks)apps/web-giddh/src/app/reports/components/payment-report/payment-report.component.html
(1 hunks)apps/web-giddh/src/app/reports/components/reverse-charge-report-component/reverse-charge-report.component.html
(4 hunks)apps/web-giddh/src/app/reports/components/reverse-charge-report-component/reverse-charge-report.component.scss
(1 hunks)apps/web-giddh/src/app/reports/components/reverse-charge-report-component/reverse-charge-report.component.ts
(4 hunks)apps/web-giddh/src/app/services/apiurls/gst-r.api.ts
(1 hunks)apps/web-giddh/src/app/settings/address-settings/address-settings.component.html
(3 hunks)apps/web-giddh/src/app/settings/address-settings/address-settings.component.ts
(1 hunks)apps/web-giddh/src/app/settings/personal-information/personal-information.component.html
(3 hunks)apps/web-giddh/src/app/settings/settings.component.html
(1 hunks)apps/web-giddh/src/app/settings/settings.component.ts
(2 hunks)apps/web-giddh/src/app/settings/taxes/confirmation/confirmation.model.component.html
(2 hunks)apps/web-giddh/src/app/settings/taxes/setting.taxes.component.html
(4 hunks)apps/web-giddh/src/app/settings/taxes/setting.taxes.component.scss
(0 hunks)apps/web-giddh/src/app/settings/taxes/setting.taxes.component.ts
(7 hunks)apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.component.html
(2 hunks)apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.component.scss
(1 hunks)apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.module.ts
(1 hunks)apps/web-giddh/src/app/shared/helpers/defaultDateFormat.ts
(1 hunks)apps/web-giddh/src/app/shared/tax-sidebar/tax-sidebar.component.ts
(1 hunks)apps/web-giddh/src/app/subscription/subscription-list/subscription-list.component.ts
(1 hunks)apps/web-giddh/src/app/theme/form-fields/input-field/input-field.component.html
(1 hunks)apps/web-giddh/src/app/theme/form-fields/input-field/input-field.component.ts
(1 hunks)apps/web-giddh/src/app/theme/tax-authority/reports/account-wise-report/account-wise-report.component.ts
(1 hunks)apps/web-giddh/src/app/theme/tax-authority/reports/rate-wise-report/rate-wise-report.component.ts
(1 hunks)apps/web-giddh/src/app/theme/tax-authority/reports/tax-authority-report/tax-authority-report.component.ts
(1 hunks)apps/web-giddh/src/app/vat-report/obligations/obligations.component.html
(1 hunks)apps/web-giddh/src/app/vat-report/obligations/obligations.component.ts
(1 hunks)apps/web-giddh/src/app/vat-report/transactions/vat-report-transactions.component.html
(0 hunks)apps/web-giddh/src/app/vat-report/transactions/vat-report-transactions.component.ts
(0 hunks)apps/web-giddh/src/app/vat-report/vat-report.component.html
(1 hunks)apps/web-giddh/src/app/vat-report/vat-report.component.scss
(0 hunks)
⛔ Files not processed due to max files limit (7)
- apps/web-giddh/src/app/vat-report/vat-report.component.ts
- apps/web-giddh/src/app/vouchers/list/list.component.ts
- apps/web-giddh/src/assets/css/global/table.scss
- apps/web-giddh/src/assets/css/style-2.scss
- apps/web-giddh/src/assets/locale/invoice/ewaybill/en.json
- apps/web-giddh/src/assets/locale/invoice/ewaybill/hi.json
- apps/web-giddh/src/assets/locale/invoice/ewaybill/mr.json
💤 Files with no reviewable changes (8)
- apps/web-giddh/src/app/vat-report/vat-report.component.scss
- apps/web-giddh/src/app/vat-report/transactions/vat-report-transactions.component.html
- apps/web-giddh/src/app/settings/taxes/setting.taxes.component.scss
- apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.scss
- apps/web-giddh/src/app/gst/gst-setting/gst-setting.component.html
- apps/web-giddh/src/app/gst/filing/tabs/overview/summary/summary.component.scss
- apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.scss
- apps/web-giddh/src/app/vat-report/transactions/vat-report-transactions.component.ts
✅ Files skipped from review due to trivial changes (14)
- apps/web-giddh/src/app/theme/tax-authority/reports/rate-wise-report/rate-wise-report.component.ts
- apps/web-giddh/src/app/gst/filing/tabs/reconcilation/reconcilation.component.scss
- apps/web-giddh/src/app/settings/address-settings/address-settings.component.ts
- apps/web-giddh/src/app/settings/settings.component.html
- apps/web-giddh/src/app/theme/tax-authority/reports/account-wise-report/account-wise-report.component.ts
- apps/web-giddh/src/app/new-inventory/component/adjust-inventory-list/adjust-inventory-list.component.ts
- apps/web-giddh/src/app/app.constant.ts
- apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.scss
- apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.component.scss
- apps/web-giddh/src/app/contact/aging-report/aging-report.component.html
- apps/web-giddh/src/app/settings/personal-information/personal-information.component.html
- apps/web-giddh/src/app/subscription/subscription-list/subscription-list.component.ts
- apps/web-giddh/src/app/reports/components/reverse-charge-report-component/reverse-charge-report.component.scss
- apps/web-giddh/src/app/theme/tax-authority/reports/tax-authority-report/tax-authority-report.component.ts
👮 Files not reviewed due to content moderation or server errors (2)
- apps/web-giddh/src/app/gst/filing/tabs/reconcilation/reconcilation.component.html
- apps/web-giddh/src/app/vat-report/vat-report.component.html
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.ts
[error] 215-215: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web-giddh/src/app/gst/filing/tabs/reconcilation/reconcilation.component.ts
[error] 158-158: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 166-166: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 174-174: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 182-182: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (105)
apps/web-giddh/src/app/gst/filing/header/filing-header.component.ts (4)
249-265
: Confirm interaction flow and ensure consistent close logic.
The new openSettingAsidePane method opens a MatDialog as an aside panel. Double-check that the user has a clear path to close or cancel the dialog. For a consistent UX, confirm that esc-key or backdrop clicks are intentionally disabled.
333-337
: Check for proper disposal or unsubscribing of dialog references.
After opening "cancelConfirmationDialog" and storing the reference in "cancelConfirmationDialogRef," ensure references are closed/destroyed properly to avoid potential memory leaks.
421-427
: Validate month/year selection edge cases.
When using setMonthAndYear, ensure the resulting date range is correct for edge cases such as leap years or time zone offsets. Also confirm that newly selected months that cross year boundaries are handled correctly.
435-439
: Ensure correct date range behavior for day boundary offsets.
dateSelected calls periodChanged with event[0]/event[1]. Confirm that hours/minutes/seconds are set to the start and end of the month as expected, especially if the user picks a date range near midnight or in a different time zone.
apps/web-giddh/src/app/gst/filing/header/filing-header.component.html (6)
3-3
: Validate usage of defensive checks for 'selectedGst'
The component checks if "selectedGst === GstReport.Gstr1." Ensure "GstReport" is defined and loaded properly. If "GstReport" is an enum or constant, make sure to handle unexpected values gracefully.
13-15
: Assess potential runtime errors in date template
The code references "showDate" and "monthSelectionDatePicker". Verify that "showDate" is correctly set to avoid accidental undefined behavior and that the template for "monthSelectionDatePicker" is always available.
26-31
: Ensure the error sheet logic is test-covered
"onDownloadSheetGSTR('error')" triggers a download of error sheets. Verify that the correct error states and data are returned from the API. You might add unit or integration tests for this behavior.
43-63
: Check module restrictions logic
Lines 58 and 59 reference restrictedModules. Ensure that plan details or subscription details are loaded consistently before rendering, to avoid potential null references or race conditions in reactive streams (like "activeCompany$").
158-165
: Consider error handling in pushToPortal flow
When pushing to the portal, handle scenarios where the push might fail (e.g., network errors, server rejections). Provide user feedback and fallback actions to maintain a smooth user experience.
167-187
: Validate date configuration for multi-year selection
The date picker uses multi-year view. Ensure that the function "setMonthAndYear" handles edge cases, such as switching years that have no data or selecting months beyond a valid range. Unit tests for these scenarios are highly recommended.
apps/web-giddh/src/app/settings/taxes/setting.taxes.component.html (4)
23-31
: Ensure ARIA labels and accessibility are fully addressed.
While the button has an aria-label attribute, confirm that the entire UI flow (including dialogs) is keyboard-accessible and that screen readers properly interpret the new “Create new tax” button. You may consider adding further ARIA attributes like roles or ensuring focus is managed appropriately when dialogs open or close.
40-45
: Index column logic looks good.
Showing a simple index is helpful for quick reference. Ensure zero-based indexing is intended. If you want the first row labeled “1,” your current logic is correct.
Line range hint 371-390
: Dialog-based aside approach is well-structured.
Using an Angular Material dialog or aside for the “Create Tax” flow improves separation of concerns. Just ensure consistent and intuitive behavior between standard dialogs and these aside panels for a coherent UX.
395-403
: Verify consistent usage of the new confirmation model.
You’ve replaced a Bootstrap-based modal with an Angular template. Double-check that all delete confirmations throughout the app now use the new approach to ensure a consistent user experience.
apps/web-giddh/src/app/financial-reports/components/filter/filter.component.html (1)
226-226
: LGTM! Simplified ngClass syntax
The ternary operator provides a cleaner way to handle conditional class binding compared to the object syntax.
apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.scss (2)
15-17
: LGTM! The new date-width class provides consistent sizing.
The minimum width of 120px is appropriate for date fields and aligns well with Material Design principles for consistent spacing.
Line range hint 1-17
: Verify mobile responsiveness after media query removal
The removal of the media query for screens ≤767px might affect the mobile layout. Please verify that the Material components properly handle the responsive behavior.
✅ Verification successful
Let me check the actual Material Design component usage and responsive elements in the template to make a more informed decision.
Material Design table and responsive utilities are properly implemented
The component uses Material Design's mat-table which has built-in responsive behavior. The table implementation includes:
- Proper usage of mat-table directives and responsive containers
- Bootstrap utility classes for alignment and spacing
- Flexible width handling with classes like
w-100
and specific column widths (e.g.,date-width
)
The removal of the media query is safe as the component relies on Material Design's responsive features and Bootstrap's utility classes rather than custom mobile breakpoints.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining mobile-specific styles and Material Design implementations
# Check for any remaining media queries in the component
rg -l "max-width: 767px" "apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/"
# Check for Material Design component usage in the template
rg -l "mat-" "apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.html"
Length of output: 304
Script:
#!/bin/bash
# Check the actual Material Design components and responsive elements in the template
rg -A 2 "mat-" "apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.html"
# Check for any Bootstrap grid classes or responsive utilities
rg -A 2 "col-[sm|md|lg|xl]|hidden-[xs|sm|md|lg]" "apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.html"
# Check if there are any global responsive styles that might affect this component
rg "max-width: 767px" "apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/"
Length of output: 6558
apps/web-giddh/src/app/reports/components/payment-report/payment-report.component.html (1)
49-49
: LGTM! Clean and simplified class binding
The simplified conditional class binding improves readability while maintaining the same functionality.
apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.ts (1)
205-213
: LGTM: Improved error handling with snackbar notifications
The error handling has been updated to use snackbar notifications with proper localization support. The validation logic is sound and provides clear user feedback.
apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.module.ts (1)
20-36
: Verify the Material conversion completeness.
The module imports look good, but since this is part of a Material UI conversion:
- Consider if you need additional Material modules like MatIconModule for icons or MatTooltipModule for tooltips
- Ensure BsDatepickerModule is intended to remain instead of MatDatepickerModule
Let's verify what Material components are being used in the template:
✅ Verification successful
Let me check the actual content of the template file to see what Material components are being used.
Based on the template content, I can now provide the final response:
Additional Material modules needed for complete Material conversion
The template shows usage of the following Material components:
mat-dialog-title
,mat-dialog-content
,mat-dialog-close
- already covered by MatDialogModulemat-stroked-button
- already covered by MatButtonModulemat-form-field
- already covered by MatFormFieldModule
The current Material imports are sufficient for the components being used in the template. The BsDatepickerModule is intentionally used for date picking functionality instead of MatDatepickerModule, as evidenced by the bsDatepicker
directive usage in the template.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check what Material components are used in the template
rg -l "mat-" "apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.component.html"
# Check for potential Material component usages that might need corresponding modules
ast-grep --pattern 'mat-$_' "apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.component.html"
Length of output: 309
Script:
#!/bin/bash
# Get the content of the template file
cat "apps/web-giddh/src/app/shared/aside-menu-create-tax/aside-menu-create-tax.component.html"
Length of output: 32688
apps/web-giddh/src/app/gst/modals/push-to-portal/push-to-portal.component.ts (3)
2-2
: LGTM! Good practice using a constant for the URL.
Using a centralized constant for the GST utility download URL improves maintainability.
Line range hint 3-9
: LGTM! Well-structured component with performance optimization.
The component follows Angular best practices with:
- OnPush change detection for better performance
- Clear input/output decorators
- Consistent method documentation
Line range hint 1-38
: Verify the impact of removing cancelCallBack.
The removal of the cancelCallBack output event might affect parent components that were previously handling cancel operations.
✅ Verification successful
The removal of cancelCallBack output event is safe
Based on the codebase analysis, the cancelCallBack
event is not used in any template where the push-to-portal
component is implemented. The only usage of this component is in apps/web-giddh/src/app/gst/filing/header/filing-header.component.html
and it only binds to localeData
input and downloadExcel
output, making the removal of cancelCallBack
safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any usage of cancelCallBack in template files
rg -t html 'cancelCallBack' apps/web-giddh/src/app/
# Search for any references in component files
rg -t ts 'cancelCallBack.*push-to-portal' apps/web-giddh/src/app/
Length of output: 3492
Script:
#!/bin/bash
# Search for any usage of push-to-portal component in template files
rg -t html '<push-to-portal' -B 2 -A 2 apps/web-giddh/src/app/
# Search for any references to push-to-portal component in TypeScript files
rg -t ts 'push-to-portal' apps/web-giddh/src/app/
# Get the previous version of the component from git history
git log -p apps/web-giddh/src/app/gst/modals/push-to-portal/push-to-portal.component.ts | grep -A 20 "@Output()"
Length of output: 1135
apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.scss (2)
42-46
: Verify the impact of removed properties
According to the AI summary, several properties were removed (background-color, padding, position, margin-top). Please ensure these removals don't adversely affect the component's layout and positioning.
✅ Verification successful
Removed properties are safely handled by parent elements and Material Dialog
The removal of background-color, padding, position, and margin-top properties from .aside-pane-form
is safe because:
-
The component's template shows the
.aside-pane-form
is wrapped inmat-dialog-content
with explicit classes handling positioning:position-relative
classpd-15
class for paddingbg-white
class for background color
-
The parent
:host
and.aside-pane
elements handle the overall positioning and layout::host
has fixed positioning.aside-pane
sets the maximum width and padding
-
The remaining properties in
.aside-pane-form
handle the essential aspects:- Box shadow for visual depth
- Height calculation for viewport fitting
- Smooth scrolling behavior
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for compensating styles in parent/child elements
# Search for related styling in Material theme files
fd -e scss -e css | xargs rg "aside-pane-form"
# Check for Material Dialog styling overrides that might affect positioning
rg "mat-dialog" --type scss
Length of output: 6350
Script:
#!/bin/bash
# Let's check the specific file for its complete styling context
cat "apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.scss"
# Check parent component's template to understand the structural context
cat "apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.html"
# Search for any global styles that might affect this component
rg "aside-pane-form" "apps/web-giddh/src/assets/css/style-2.scss"
Length of output: 18733
44-44
: Verify the impact of reduced height on content visibility
The height calculation has been reduced by 25px (from 90px to 65px). This change might affect content visibility and scrolling behavior within the aside menu.
✅ Verification successful
Let me gather more information about the component's layout and scroll handling.
Height adjustment is properly handled with overflow
The height reduction from 90px to 65px is safe because:
- The component has
overflow-auto
class on the content container - The content area has proper scroll behavior with
scroll-behavior: smooth
- The layout maintains responsiveness across different breakpoints
- The height calculation accounts for the header height while allowing content to scroll
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential content overflow issues due to height reduction
# Search for content elements within the aside menu template
rg -l "aside-pane-form" --type html
# Check for any scroll-related styles or overflow handling
ast-grep --pattern 'overflow: $value;'
ast-grep --pattern 'overflow-y: $value;'
Length of output: 2171
Script:
#!/bin/bash
# Check the template and styles of the GST aside menu component
cat apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.html
cat apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.scss
# Check for any scroll-related classes or styles in the component
rg -A 3 "overflow|scroll" apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.*
Length of output: 19810
apps/web-giddh/src/app/models/api-models/Invoice.ts (1)
Line range hint 642-652
: Well-structured classes with proper types
The InvoicePreviewDetailsVm
and InvoicePaymentRequest
classes are well-designed with:
- Proper use of specific types
- Consistent usage of optional properties
- Good use of existing interfaces
- Appropriate use of union types for dates
Also applies to: 654-663
apps/web-giddh/src/app/reports/components/reverse-charge-report-component/reverse-charge-report.component.ts (2)
90-106
: LGTM! Well-structured material table configuration and search properties
The new properties are well-defined and follow Angular's naming conventions. The material table columns are properly structured.
262-269
: LGTM! Correct handling of Material's zero-based pagination
The new implementation properly handles Material's zero-based pagination by adding 1 to the pageIndex.
apps/web-giddh/src/app/gst/filing/tabs/push-to-gstin/components/hsn-summary/hsn-summary.component.ts (1)
44-46
: 🛠️ Refactor suggestion
Consider improving column handling logic.
The current implementation has several potential issues:
- Directly modifying displayedColumns could cause issues if the component reinitializes.
- The comparison lacks type safety.
Consider this improved implementation:
+private readonly baseColumns: string[] = [
+ 'hsn_sc', 'desc', 'qty', 'uqc', 'txval', 'rt',
+ 'iamt', 'camt', 'samt', 'csamt', 'val'
+];
+
+public displayedColumns: string[] = [];
+
public ngOnInit() {
this.imgPath = isElectron ? 'assets/images/gst/' : AppUrl + APP_FOLDER + 'assets/images/gst/';
- if (this.selectedGst !== GstReport.Gstr1) {
- this.displayedColumns = this.displayedColumns.filter(column => column !== 'rt');
- }
+ this.displayedColumns = this.baseColumns.filter(column =>
+ this.selectedGst === GstReport.Gstr1 || column !== 'rt'
+ );
}
This approach:
- Preserves the original column configuration
- Makes the filtering logic more explicit
- Ensures clean reinitialization
Let's verify the usage of GstReport.Gstr1 across the codebase:
apps/web-giddh/src/app/theme/form-fields/input-field/input-field.component.ts (1)
85-90
: Verify usage of the new icon properties
Let's ensure the new properties are being used correctly across the codebase.
✅ Verification successful
Icon properties are used correctly with proper accessibility context
The verification shows that:
- The new icon properties are being used in
gstR3.component.html
with proper implementation - The icon has an associated tooltip (
labelIconTooltip
) bound to localized content (localeData?.gstr3b?.email_tooltip
) - Default values are sensibly set in the component (info circle icon and disabled by default)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of input-field component with the new icon properties
rg -l 'input-field' | xargs rg -A 5 -B 5 'showLabelIcon|labelIconClass|labelIconTooltip'
# Search for potential icon-related accessibility issues
ast-grep --pattern 'matTooltip|aria-label|role="img"'
Length of output: 3605
apps/web-giddh/src/app/gst/modals/gst-aside-menu/gst-aside-menu.component.ts (1)
131-131
: Verify if limiting provider options to only TAXPRO is intentional
The component supports multiple providers (TAXPRO, VAYANA) in its business logic, but the UI only shows TAXPRO as an option. This mismatch could cause confusion or limit functionality.
Consider maintaining consistency between UI options and supported providers. If VAYANA support is being deprecated, ensure all related code is cleaned up. If not, consider adding it to the provider options.
apps/web-giddh/src/app/gst/filing/tabs/push-to-gstin/push-to-gstin.component.html (2)
117-117
: LGTM on the loader change!
The replacement of the spinner with <giddh-page-loader>
aligns with the material design transition.
Let's verify the component's availability and implementation:
✅ Verification successful
Based on the search results, I can see that the giddh-page-loader
component is widely used across the application and is properly implemented. The key findings are:
- The component exists in
apps/web-giddh/src/app/shared/giddh-page-loader/giddh-page-loader.component.ts
- It has its own module
GiddhPageLoaderModule
for proper Angular module organization - It's imported and used correctly across many components in the application
- The component is used consistently for loading states
Let me verify the component's implementation to ensure it's properly defined:
LGTM! The loader component is properly implemented and available
The <giddh-page-loader>
component is correctly implemented with:
- A proper Angular component structure
- Support for custom CSS classes via
@Input()
- A material design-style loading animation using cubes
- Consistent usage across the application
The change aligns with the material design transition and maintains proper loading state functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the giddh-page-loader component exists and is properly implemented
rg -g '*.ts' -g '*.html' "giddh-page-loader" -A 5
Length of output: 65586
Script:
#!/bin/bash
# Check the component implementation
cat apps/web-giddh/src/app/shared/giddh-page-loader/giddh-page-loader.component.ts
cat apps/web-giddh/src/app/shared/giddh-page-loader/giddh-page-loader.component.html
Length of output: 647
Line range hint 25-115
: Critical: Large sections of commented code need attention
The file contains extensive commented-out sections including crucial functionality (transaction summary, HSN, NIL, B2CS, document issued, and failed transactions tabs). This appears to be incomplete refactoring and could impact the module's functionality.
Questions to address:
- Are these tabs being replaced with material equivalents?
- Is the functionality being moved elsewhere?
- If these features are no longer needed, should the code be removed instead of commented?
Let's check if these features exist elsewhere or are being replaced:
apps/web-giddh/src/app/settings/address-settings/address-settings.component.html (1)
1-2
: LGTM: Proper overflow handling for Material components
The addition of overflow-visible
class ensures proper rendering of Material Design dropdowns and tooltips.
apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.html (1)
Line range hint 1-164
: Verify Material Design module imports
Please ensure that all required Material Design modules are properly imported in the corresponding module file.
✅ Verification successful
Material Design modules are properly configured
The review comment's concern about Material Design imports has been verified. The invoice.module.ts
file shows that all necessary Material modules are properly imported and included in the module's imports array, including:
- MatDialogModule (for dialog functionality)
- MatTabsModule (for tab components)
- MatFormFieldModule (for form fields)
- MatInputModule (for input fields)
- Other required Material modules for the component's functionality
The template's usage of mat-dialog-content
is supported by these imports, and no issues were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if MaterialModule is imported in the feature module
rg -l "MaterialModule" "apps/web-giddh/src/app/invoice"
# Check for individual Material imports
ast-grep --pattern 'import { Mat$_ } from "@angular/material/$_"'
Length of output: 121
Script:
#!/bin/bash
# Let's check for Material Dialog imports and usage in the component
rg -A 2 "MatDialogModule|MatDialog" "apps/web-giddh/src/app/invoice"
# Check the component's TypeScript file for Material imports
rg "import.*@angular/material" "apps/web-giddh/src/app/invoice/preview/models/download-or-send-mail/download-or-send-mail.component.ts"
# Check the module file for Material imports
fd "invoice.module.ts" "apps/web-giddh/src/app/invoice" --exec cat {}
Length of output: 14154
apps/web-giddh/src/app/reports/components/reverse-charge-report-component/reverse-charge-report.component.html (2)
28-34
: Migrating to looks good
Replacing with is consistent with your transition to Angular Material-like components. This improves code uniformity and is aligned with your app-wide modernization efforts.
625-654
: Reusable search template approach
The new #searchTemplate highlights a clean, reusable approach to table column searches. This improves maintainability and consistency.
apps/web-giddh/src/app/gst/modals/push-to-portal/push-to-portal.component.html (2)
33-48
: Apply the same loading state improvements as suggested for the Excel download button
8-31
: 🛠️ Refactor suggestion
Add loading states and error handling
The download functionality should handle loading states and potential errors:
<button
mat-stroked-button
color="primary"
class="downloads-button"
- (click)="onDownloadExcel($event)"
+ (click)="onDownloadExcel($event)"
+ [disabled]="isDownloading"
>
- {{ localeData?.filing?.download_excel }}
+ <mat-spinner *ngIf="isDownloading" diameter="20"></mat-spinner>
+ <span>{{ localeData?.filing?.download_excel }}</span>
</button>
Also consider adding error handling for the GST utility download link:
-<a class="mr-0 downloads-button json-button" target="_blank" [href]="gstUtilityDownloadLink">
+<a class="mr-0 downloads-button json-button" target="_blank" [href]="gstUtilityDownloadLink" (error)="handleDownloadError($event)">
apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.ts (9)
26-27
: Use of Angular Material dialog components is good for consistent UI patterns.
Nicely done importing MatDialog and MatDialogRef. However, double-check that any old references (e.g., BsModalService) that are no longer needed are removed or consolidated to avoid code duplication.
142-149
: Helpful clarity using additional class properties.
Declaring separate variables for tab tracking (activeTabIndex, selectedTab), displayedColumns, and a MatDialogRef is a good step toward clarity. Just keep an eye on memory usage and potential race conditions if multiple dialogs could open simultaneously.
218-218
: Null-safe usage of cancelDialogRef.
Using the optional chaining operator for close() is good for preventing runtime errors.
333-333
: Branch alias property.
Properly assigning alias from the active company is good; verify that it’s not overwritten if the user changes the branch.
464-474
: Method openCancelDialog.
Similar to openAddVehicleDialog, you store the opened dialog in cancelDialogRef. Check for consistent usage patterns (observation of afterClosed, etc.).
588-588
: Persisting GSTIN filter in the model.
Good job ensuring gstin is present in the final model. Watch out for scenarios where multiple GSTINs are used, ensuring the code handles them effectively.
229-230
: Cloning eWayBillList results.
Cloning the object ensures immutability. Double-check that existing references to these results don’t rely on direct mutation.
✅ Verification successful
No mutation concerns with ewaybillLists results
The deep clone followed by reassignment is safe. The ewaybillLists.results
is only used for read-only operations in the template for data display purposes (*ngFor
, [dataSource]
, and length checks). There are no direct mutations of the array after assignment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
#!/bin/bash
# Search for direct usage of `this.ewaybillLists.results` to confirm no direct array manipulation
rg "ewaybillLists\.results(\s+)?="
Length of output: 163
Script:
#!/bin/bash
# Let's check for any other references or mutations of ewaybillLists.results
rg "ewaybillLists\.results" -A 2 -B 2
Length of output: 567
Script:
#!/bin/bash
# Let's check how ewaybillLists is used throughout the component
rg "ewaybillLists" -A 3 -B 3
Length of output: 5289
190-190
: Assigning GSTIN automatically.
Ensuring that gstin is set using the active company’s response is good. Confirm that other components or services don’t override or conflict with this value.
✅ Verification successful
Let me analyze the code further to understand the GSTIN assignment flow.
Based on the analysis of the code, I can now generate the final response:
GSTIN assignment is properly handled without conflicts
The GSTIN assignment in EwayBillfilterRequest is well-managed with clear assignment flows:
- Initially set from the active company's response
- Falls back to first tax value if not set
- Can be manually selected through the dropdown
- No conflicting overwrites found in other components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
#!/bin/bash
# Check for possible overwrites of EwayBillfilterRequest.gstin
rg -A 5 "EwayBillfilterRequest\.gstin"
Length of output: 3129
Script:
#!/bin/bash
# Check for the EwayBillfilterRequest class/interface definition
ast-grep --pattern 'interface EwayBillfilterRequest {
$$$
}'
# Also check for any other components or services that might be using this request
rg -A 3 "EwayBillfilterRequest"
Length of output: 11222
54-54
: Renamed ewaybillLists property.
Good practice aligning the property name with coding conventions (lower camelCase). Verify that all references to the old eWaybillLists are updated.
✅ Verification successful
Based on the search results, I can see that the property ewaybillLists
is only used in its new camelCase form in the component file and there are no references to any old PascalCase version (eWaybillLists
). The property is correctly used in its camelCase form in the component's code:
- Declaration:
public ewaybillLists: IEwayBillAllList;
- Usage in subscription:
this.ewaybillLists = _.cloneDeep(o);
- Usage in assignment:
this.ewaybillLists.results = o.results;
The property name follows camelCase convention and all references are consistent.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify usage of 'ewaybillLists' and ensure no references to old property name remain.
rg "eWaybillLists" -A 5
Length of output: 23
Script:
#!/bin/bash
# Search for both camelCase and PascalCase variations of the property name
rg -i "ewaybilllists?" --type ts -A 5
Length of output: 8467
Script:
#!/bin/bash
# Search for property declarations and usages in the component file
ast-grep --pattern 'ewaybillLists = $_'
ast-grep --pattern 'this.ewaybillLists'
Length of output: 340
Script:
#!/bin/bash
# Get the content of the component file to understand the context
cat "apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.ts"
Length of output: 31857
apps/web-giddh/src/app/gst/filing/tabs/push-to-gstin/components/hsn-summary/hsn-summary.component.html (3)
7-7
: Ensure fallback handling for data source
Using hsnSummary.data directly is fine if always defined. However, consider implementing a safe fallback in case hsnSummary might be null or undefined:
Could you ensure hsnSummary.data always exists (for instance, set it to an empty array when undefined)?
40-43
: Conditional column usage
The conditional display for Rate (rt) column is correct. Just be sure your logic for selectedGst can’t result in partial states (e.g., if selectedGst changes asynchronously) causing sporadic toggling.
80-81
: Verify displayedColumns presence
Ensure displayedColumns is declared and updated in the component’s TypeScript file to match these matColumnDef identifiers. Omitting any column or mismatch in naming can break rendering.
apps/web-giddh/src/app/gst/filing/filing.component.html (2)
63-74
: Ensure reconcilation tab logic
Line 63 restricts the entire tab to Gstr2 or Gstr1. Double-check whether you need to handle future additions or states (e.g., Gstr3 might also require reconcilation). Maintaining a single condition can become more challenging as your application evolves; consider a more flexible approach or a dynamic list of valid GST reports.
75-96
: User flow check for fileReturn
The condition for “File Return” tab is gating based on isCompany, selectedGst, and showGstFiling. If any of these flags are toggled quickly or from an async source, the template can appear midway. Relying on multiple boolean conditions can lead to confusion. Test thoroughly for all states.
apps/web-giddh/src/app/gst/filing/tabs/overview/summary/summary.component.html (1)
175-186
: Clickable row logic
Your row click logic (line 177) calls viewTransactions(row). If row.transactions is present, you set a no-hover style. Verify that the intended behavior is correct (the code indicates “no-hover” if there’s a transactions array, but the label says 'cursor-pointer' if it’s not). This can be inverted from typical user expectations.
apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.html (4)
68-72
: Verify data existence
You’re conditionally showing the table if (viewTransaction$ | async)?.results?.length is true. Ensure you handle partial states: for example, if results is present but totalItems is 0 or vice versa.
74-88
: Invoice invocation
You use (click)="onSelectInvoice(item)" to handle invoice selection for any item not marked “purchase.” Double-check your TypeScript logic for onSelectInvoice to confirm it gracefully handles all voucher types.
239-250
: Pagination
Using mat-paginator is a great improvement. For consistency, confirm that pageChanged accurately updates pageIndex and pageSize in the TS code so that subsequent fetches for the next pages behave properly.
274-286
: Modal approach
You introduced a separate downloadOrSendMailDialog template. This is good for modularizing the UI. Confirm that the component’s module (likely a shared or a feature module) declares the DownloadOrSendMailInvoice component and has the MatDialogModule imported. Missing declarations can cause runtime errors.
apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.ts (6)
156-156
: **Pagination default aligns with pageSizeOptions **
Setting the filterParam.count to the first index of pageSizeOptions is good. This ensures that your default pagination size matches your defined constant.
192-203
: **Good approach to page-changed logic **
Your updated “pageChanged” method cleanly updates filterParam and calls “viewFilteredTxn” with the correct page index.
234-234
: **Safe dialog closing **
Using the optional chaining operator is prudent here (downloadOrSendMailDialogRef?.close()) to avoid potential undefined references.
366-366
: **Uniform fallback messages **
Dispatching “app_something_went_wrong” consistently ensures minimal user confusion.
96-101
: **Ensure user-friendly pagination options **
The new pagination variables (pageIndex, pageSizeOptions, downloadOrSendMailDialogRef) look fine. Verify that no older references to a different pagination approach remain.
✅ Verification successful
The search results show that the pagination variables (pageIndex
, pageSizeOptions
, downloadOrSendMailDialogRef
) in the view-transactions component are consistently used with Angular Material's mat-paginator across multiple components in the codebase. The implementation follows the same pattern as other components using pagination, such as:
- The same variable names and types are used in other components (e.g., rate-wise-report, tax-authority-report, etc.)
- The variables are used with mat-paginator in the same way in template files
- The page change handling logic is consistent across components
Pagination implementation is consistent with codebase standards ✓
The new pagination variables follow the established pattern used throughout the application and integrate properly with Angular Material's paginator component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching references to old pagination variables across the codebase
rg "pageIndex|pageSizeOptions" -A 5
Length of output: 58182
249-249
: **Dynamic columns insertion **
Adding “voucherType” column at index 4 might shift the table layout significantly if other code relies on the displayedColumns order. Verify that subsequent usage is updated accordingly.
✅ Verification successful
Let me analyze the code further to verify the impact of adding the 'voucherType' column.
Let me check how the displayedColumns are used in the template and if there are any dependencies on the column order.
Based on the analysis of the code, I can now provide the final response:
**Column insertion is safe **
The insertion of 'voucherType' column at index 4 is safe because:
- The table uses Angular Material's matColumnDef which renders columns based on their definition order, not array indices
- Each column has its own independent template definition
- The column values are accessed by name, not by position
- No other code relies on specific column positions for calculations or UI rendering
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Checking other references of displayedColumns to see if insertion breaks UI
rg "displayedColumns"
Length of output: 44829
Script:
#!/bin/bash
# Check the view-transactions component implementation and template
rg -A 10 "displayedColumns.*=" "apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.ts"
Length of output: 401
Script:
#!/bin/bash
# Check the template usage of displayedColumns and column definitions
rg -A 20 "mat-(header|row|cell)" "apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.html"
Length of output: 9423
apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.html (6)
61-91
: **Material Table usage **
Using mat-table and matColumnDef is a neat approach. The columns index, invoiceDate, docNumber, etc., align well with standard Angular Material guidelines.
103-112
: **Indicator for manually generated eWay Bills **
Displaying “manually_generated” as a small text with a distinctive class is a good way to highlight such data.
135-156
: **Actions column with mat-menu **
This approach keeps your UI tidy, offering multiple actions under a single button. The code is clear, but do re-check that each action is user-friendly and self-explanatory.
168-168
: **Show “no-data” when array is empty **
Providing a fallback with fosters good UX.
Line range hint 266-290
: **Universal Datepicker template **
Your approach in separating inside an ng-template fosters reusability.
240-240
: **Double-check consolidated table usage **
In #consolidateModal, you’re iterating over “ewaybillLists?.results”. Confirm it is the intended data source for the consolidated details if that block is truly not used. If it’s “Not In Use,” consider removing or gating it behind a feature flag.
✅ Verification successful
Let me gather more context about the usage of this modal and the data source.
Let me check more context about the actual usage of this modal and the table structure.
Based on the gathered context, I can now provide a final response:
**The consolidated modal and table are actively used features **
The code shows this is an actively used feature for consolidated branch functionality. The ewaybillLists?.results
is correctly used to display e-way bill details in a consolidated view, with proper integration with branch consolidation state management (isConsolidatedBranch
). The table displays important e-way bill information including invoice date, document number, customer details, and e-way bill numbers. No removal or feature flag is needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
rg -A 5 "consolidateModal"
Length of output: 724
Script:
#!/bin/bash
# Check the component file for usage of consolidateModal and ewaybillLists
rg -A 10 "ewaybillLists|consolidateModal" "apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.ts"
# Get more context around the template usage
rg -A 15 "consolidateModal" "apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.html"
Length of output: 2353
Script:
#!/bin/bash
# Get the full template content around the table with ewaybillLists
rg -B 5 -A 20 "tr \*ngFor=\"let item of ewaybillLists\?.results\"" "apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.html"
# Check if there are any methods related to consolidated functionality
rg -A 5 "consolidate|Consolidate" "apps/web-giddh/src/app/invoice/eWayBill/eWayBill/eWayBill.component.ts"
Length of output: 3027
apps/web-giddh/src/app/gst/gst.component.html (5)
66-83
: **Material Datepicker: month selection **
Using (monthSelected) is a clean pattern. The date manipulation is done in your TypeScript code, which is recommended for clarity.
101-115
: **Conditional UI for GSTR3B summary **
Applying “content-disabled” style if no activeCompanyGstNumber is a nice way to unify your UI state. The approach here is consistent with other placeholders.
282-315
: **Refine unreachable code for gstr2? **
The “gstr2TransactionCounts” block includes an “uncategorized” section set to zero. If it’s always zero, confirm we need to keep it.
335-347
: **“gstr2_push_disabled” usage **
Clear indication to the user that GSTR2 push is disabled. The icon usage is straightforward with a consistent color scheme.
37-44
: **Switch from sh-select to dropdown-field: verify options **
Confirm that the “(selectedOption)” event binding logic matches the older custom select. The new “defaultValue” binding must ensure the correct default GSTIN is applied.
✅ Verification successful
Let me analyze the dropdown-field component usage to verify the event binding and default value handling. I'll search for the component definition and its usage.
Let me check the dropdown-field component implementation to verify the event binding.
Based on the code analysis, I can now generate the final response:
Event binding and default value handling are correctly implemented
The dropdown-field component properly supports both the "(selectedOption)" event binding and "[defaultValue]" input that were used in the GST component. The implementation shows:
@Output() selectedOption
emits selected values throughoptionSelected()
method@Input() defaultValue
handles pre-selected values- The GST component's
selectTax()
method correctly processes the emitted value
The switch from sh-select to dropdown-field maintains the same functionality with proper event handling and default value support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
rg "sh-select|dropdown-field"
Length of output: 80919
Script:
#!/bin/bash
# Search for dropdown-field component definition
ast-grep --pattern 'export class DropdownFieldComponent {
$$$
}'
# Search for selectTax method in gst component
rg "selectTax" -A 5 "apps/web-giddh/src/app/gst/gst.component.ts"
Length of output: 372
Script:
#!/bin/bash
# Search for dropdown-field component implementation
rg -A 20 "@Output\(\)\s+selectedOption|@Input\(\)" "apps/web-giddh/src/app/theme/form-fields/dropdown-field/dropdown-field.component.ts"
# Check the template file for event binding
cat "apps/web-giddh/src/app/theme/form-fields/dropdown-field/dropdown-field.component.html"
Length of output: 11920
apps/web-giddh/src/app/gst/gstR3/gstR3.component.ts (6)
19-21
: **Leverage standard Angular libraries **
Including BsModalRef, BsModalService, FormControl, and MatDatepicker indicates you’re aligning with widely used, well-supported libraries.
70-88
: **Displayed columns arrays **
Defining arrays for each table within the component is a neat approach. Good job systematically labeling them for GSTR user table data, GSTR3B, ITC, etc.
97-98
: **Well-organized constructor **
Injecting generalService and modalService last (or grouped) is consistent with your logic flow. The approach is clear.
223-230
: **Batch table data setup after receiving gstr3BData **
Your approach calling setGstrUserTableData, setGstr3bTableData, etc., in sequence after response is straightforward. This ensures the UI updates happen once data is available.
593-606
: **Date logic in periodChanged **
Capturing both GSTR1, GSTR2, and GSTR3b overview inside periodChanged is consistent. Good job ensuring that requests are dispatched for all relevant types.
63-68
: **New date-related variables **
You introduced “customMonth,” “startAt,” and a FormControl “date.” Verify that these remain in sync from UI interactions (e.g., resets, clearing, or dynamic changes).
✅ Verification successful
Let me analyze the code further to verify the synchronization between these date-related variables.
Let me analyze the template to see how these variables are used in the UI and verify their synchronization.
Based on the code analysis, I can now provide a final response about the synchronization of these date-related variables.
**Date variables are properly synchronized **
The code shows proper synchronization between customMonth
, startAt
, and date
FormControl:
- The
dateSelectedEvent
method handles the date picker selection and updates all three variables in sync customMonth
is set with formatted date stringdate
FormControl is updated with thecustomMonth
value- The variables are used correctly in the template with proper bindings
- The synchronization follows the same pattern used in other similar components in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
rg "customMonth|startAt|new FormControl" -A 5
Length of output: 31145
Script:
#!/bin/bash
# Let's check for the methods that handle these variables
ast-grep --pattern 'customMonth = $_' -l ts
rg "customMonth|startAt|date\." -B 2 -A 5 apps/web-giddh/src/app/gst/gstR3/gstR3.component.ts
Length of output: 2158
Script:
#!/bin/bash
# Check the template usage of these variables
rg -B 2 -A 2 "\[startAt\]|\[formControl\]=\"date\"|\{\{customMonth\}\}" apps/web-giddh/src/app/gst/gstR3/gstR3.component.html
Length of output: 405
apps/web-giddh/src/app/contact/contact.component.ts (2)
418-422
: Validate multi-subscription workflow.
This block checks and assigns isPlaidSupportedCountry within another subscription. Ensure all subscription lifecycles are well-managed to avoid memory leaks, either via the destroyed$ subject or the async pipe.
450-450
: No issues found in detectChanges call.
This line looks fine; you are rightly ensuring the view reflects state changes.
apps/web-giddh/src/app/shared/helpers/defaultDateFormat.ts (1)
8-9
: Ensure consistent usage of newly introduced date formats.
The new date formats (TIME and MONTH_YEAR) look good. Verify that places expecting 12-hour or 24-hour time formatting are correct, and that “MMMM YYYY” is actually used or tested in relevant date-pickers and reports.
apps/web-giddh/src/app/settings/taxes/confirmation/confirmation.model.component.html (1)
6-19
: Good adaptation to Material Dialog structure.
Using and for the Yes/No prompts aligns with Material guidelines. Confirm you have theming consistent with your design system.
apps/web-giddh/src/app/services/apiurls/gst-r.api.ts (1)
7-7
: Good addition of the “count” parameter for pagination.
Ensure all callers of GET_TRANSACTIONS now pass a valid “count” to avoid unexpected API responses or default behaviors.
apps/web-giddh/src/app/gst/unit-mapping/unit-mapping.component.html (2)
1-6
: LGTM: Translation implementation
The translation setup with appTranslate
directive and proper event bindings for locale data looks good.
31-37
: Verify dropdown field behavior with empty/null values
The dropdown-field implementation looks good with proper bindings. The null coalescing operator (??
) for defaultValue provides good protection against null/undefined values. However, please verify:
- The dropdown behavior when no value is selected
- The handling of invalid mappedGstUnit values
✅ Verification successful
Let me search for the dropdown-field component implementation to verify its behavior with empty/null values.
Dropdown field handles empty/null values appropriately
After reviewing the dropdown-field component implementation, I can confirm that it properly handles empty/null/invalid values:
-
For empty/null selections:
- When no value is selected, it emits
{ label: '', value: '' }
after blur - The null coalescing operator (
??
) in the template safely handles undefined mappedGstUnit
- When no value is selected, it emits
-
For invalid values:
- The component properly filters and validates options
- Default value changes are properly handled in ngOnChanges
- The component maintains type safety through the IOption interface
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for dropdown-field usage patterns and error handling
ast-grep --pattern 'dropdown-field [
$$$
(selectedOption)="$_"
$$$
]'
Length of output: 77
Script:
#!/bin/bash
# Search for dropdown-field component definition and implementation
fd -e ts -e html "dropdown-field" --exec rg -l "dropdown-field"
# Search for specific handling of null/undefined values in dropdown-field
rg -i "dropdown.*field.*null|dropdown.*field.*undefined" -A 5
# Look for the dropdown-field component class
ast-grep --pattern 'class $_ implements $_ {
$$$
@Input() defaultValue
$$$
}'
Length of output: 401
Script:
#!/bin/bash
# Check the implementation of dropdown-field component
cat ./apps/web-giddh/src/app/theme/form-fields/dropdown-field/dropdown-field.component.ts
# Check for error handling or validation in the component
rg -A 5 "selectedOption.emit" ./apps/web-giddh/src/app/theme/form-fields/dropdown-field/dropdown-field.component.ts
Length of output: 13044
apps/web-giddh/src/app/gst/filing-status/filing-status.component.html (1)
64-75
: Verify pagination configuration
The Material paginator implementation looks good, but please verify:
✅ Verification successful
Let me gather more information about the pageChanged handler since it wasn't found in the previous search.
Let me check the implementation of the pageChanged handler to verify it properly handles pagination events.
Pagination implementation is correctly configured
The pagination implementation is properly configured with:
- Correct handling of page events in
pageChanged
method - Proper synchronization between
pageIndex
,pageSize
, and pagination state - Consistent use of
showFirstLastButtons
andpageSizeOptions
- Appropriate triggering of data refresh via
getGstrReferences()
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent pagination configuration across components
rg -A 5 "mat-paginator" | grep -E "showFirstLastButtons|pageSizeOptions|pageIndex"
# Check if pageChanged handler exists in component
ast-grep --pattern 'pageChanged($_) {
$$$
}'
Length of output: 11615
Script:
#!/bin/bash
# Search for pageChanged handler in the filing-status component specifically
rg -t ts "pageChanged" apps/web-giddh/src/app/gst/filing-status/
# Check the component class implementation
ast-grep --pattern 'export class FilingStatusComponent {
$$$
}'
Length of output: 243
Script:
#!/bin/bash
# Get the implementation of pageChanged handler and surrounding context
rg -A 10 "pageChanged" apps/web-giddh/src/app/gst/filing-status/filing-status.component.ts
# Check pagination-related variable declarations
rg "pageSizeOptions|pageIndex|pagination" apps/web-giddh/src/app/gst/filing-status/filing-status.component.ts
Length of output: 1073
apps/web-giddh/src/app/gst/gst.module.ts (1)
55-61
: Consider lazy loading Material modules
The addition of multiple Material modules might increase the initial bundle size. Consider lazy loading these modules where possible.
✅ Verification successful
Material modules are appropriately used and lazy loaded
The GstModule is already lazy loaded via the route configuration in app.routes.ts
. The Material components (dialogs, tabs, etc.) are actively used across multiple components within the GST feature module, as evidenced by their usage in templates and component files. Since these Material components are essential for the GST module's functionality and the module itself is lazy loaded, the current implementation follows best practices for optimizing bundle size.
- Material Dialog is used in view-transactions, filing-header, and gst-setting components
- Material components are used across 10 different template files within the GST module
- The module is lazy loaded through the 'gstfiling' route
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check bundle size impact
rg -l "MatSelect|MatTabs|MatDialog|MatCheckbox|MatPaginator|MatMenu|MatList" "apps/web-giddh/src/app/gst/"
# Check if these modules are used in lazy-loaded routes
rg "loadChildren.*GstModule" "apps/web-giddh/src/app/"
Length of output: 739
Script:
#!/bin/bash
# Let's check the actual usage of Material components in these files
rg -A 5 "MatSelect|MatTabs|MatDialog|MatCheckbox|MatPaginator|MatMenu|MatList" "apps/web-giddh/src/app/gst/gst-setting/gst-setting.component.ts" "apps/web-giddh/src/app/gst/filing/header/filing-header.component.ts" "apps/web-giddh/src/app/gst/filing/tabs/overview/view-transactions/view-transactions.component.ts"
# Check if these components are used across multiple templates
fd . apps/web-giddh/src/app/gst/ -e html -x rg -l "mat-select|mat-tab|mat-dialog|mat-checkbox|mat-paginator|mat-menu|mat-list"
Length of output: 7883
apps/web-giddh/src/app/company-import-export/component/form/company-import-export-form.html (1)
28-28
: LGTM! Improved ngClass readability
The simplified ternary operator usage in ngClass improves code readability while maintaining the same functionality.
apps/web-giddh/src/app/gst/filing/tabs/overview/summary/summary.component.ts (2)
52-53
: LGTM! Well-structured table columns definition.
The column names are descriptive and cover all necessary fields for GST summary display.
70-71
: LGTM! Robust subscription handling with proper cleanup.
The code demonstrates good practices:
- Null checks prevent runtime errors
- Consistent use of takeUntil for subscription management
- Clear conditional checks for GST report types
Also applies to: 75-77
apps/web-giddh/src/app/vat-report/obligations/obligations.component.html (1)
7-11
: Verify the impact of removing clickOutside event handler.
The removal of clickOutside event might affect the sidebar's closing behavior on mobile devices.
✅ Verification successful
Removal of clickOutside is safe - mobile handling is preserved
The tax-sidebar component still maintains proper mobile handling through the closeAsidePaneIfMobile()
method which is triggered by the clickOutside event. The component checks for mobile screen and handles closing appropriately:
public closeAsidePaneIfMobile(event?): void {
if (this.isMobileScreen && event && event.target.className !== "icon-bar") {
this.closeAsideEvent.emit(event);
}
}
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for mobile-specific handling in the component
rg -A 5 "isMobileScreen|mobile" apps/web-giddh/src/app/vat-report/obligations/
Length of output: 80
Script:
#!/bin/bash
# Let's check the tax-sidebar component implementation for mobile handling
ast-grep --pattern 'class TaxSidebarComponent {
$$$
}'
# Also search for any click outside handling in tax-sidebar
rg -A 5 "clickOutside|handleOutsideClick"
Length of output: 71037
Script:
#!/bin/bash
# Let's check the tax-sidebar component implementation for mobile handling
rg -A 5 "closeAsidePaneIfMobile" apps/web-giddh/src/app/shared/tax-sidebar/
# Also check if there are any other mobile-specific handlers in the component
rg -A 5 "isMobile|mobile|screen" apps/web-giddh/src/app/shared/tax-sidebar/
Length of output: 3344
apps/web-giddh/src/app/gst/gst.component.scss (1)
451-451
: LGTM! Minor formatting change.
The media query structure is maintained correctly.
apps/web-giddh/src/app/gst/filing-status/filing-status.component.ts (1)
63-73
: LGTM: Pagination properties are well-structured
The pagination implementation using Material Design's properties follows Angular Material best practices.
apps/web-giddh/src/app/models/api-models/GstReconcile.ts (1)
116-116
: LGTM: Count property addition
The addition of the count property to GStTransactionRequest class is consistent with the existing pattern and supports the new pagination implementation.
apps/web-giddh/src/app/settings/taxes/setting.taxes.component.ts (1)
66-67
: LGTM! Well-structured table configuration
The MatTableDataSource setup and column definitions are clear and comprehensive.
apps/web-giddh/src/app/home/components/cr-dr-list/cr-dr-list.component.ts (1)
107-107
: LGTM! Simplified condition
The removal of unnecessary optional chaining operator improves code clarity while maintaining the same functionality.
apps/web-giddh/src/app/gst/gstR3/gstR3.component.scss (1)
219-219
: LGTM: Layout adjustment for better content display.
The increase in max-width from 930px to 1085px appears appropriate for accommodating more content while maintaining the layout structure.
apps/web-giddh/src/app/shared/tax-sidebar/tax-sidebar.component.ts (2)
217-220
: LGTM: Improved tax selection logic.
The condition change from exact match to length > 0
makes the logic more flexible, and the addition of selectTax()
ensures proper state management.
227-235
: LGTM: Well-structured state management implementation.
The new selectTax
method is properly documented and follows Redux/NgRx patterns for state management.
apps/web-giddh/src/app/gst/filing/filing.component.ts (3)
15-15
: LGTM: Clean migration to Angular Material tabs.
The addition of MatTabChangeEvent and activeTabIndex property properly supports the migration to Material Design tabs.
Also applies to: 66-67
154-154
: LGTM: Simplified tab selection logic.
The direct assignment to activeTabIndex simplifies the code while maintaining the same functionality.
298-308
: LGTM: Well-implemented tab change handler.
The new onTabChange method is properly typed, includes appropriate null checks, and maintains both the active index and selected tab label.
<button | ||
mat-stroked-button | ||
color="primary" | ||
aria-labelledby="button-triggers-manual" | ||
[matMenuTriggerFor]="downloadMenu" | ||
> | ||
<i class="icon-download"></i> | ||
{{ commonLocaleData?.app_download }} | ||
</button> | ||
<ul id="dropdown-menu" *dropdownMenu class="dropdown-menu dropdown-menu-right" role="menu" | ||
aria-labelledby="button-triggers-manual"> | ||
<li role="menuitem"> | ||
<a class="dropdown-item" href="javascript:void(0);" aria-label="download" | ||
(click)="onDownloadSheetGSTR('main')">{{ | ||
localeData?.filing?.download_sheet | ||
}}</a> | ||
</li> | ||
<li role="menuitem"> | ||
<a class="dropdown-item" href="javascript:void(0);" aria-label="download" | ||
(click)="onDownloadSheetGSTR('error')">{{ | ||
localeData?.filing?.download_error_sheet | ||
}}</a> | ||
</li> | ||
</ul> | ||
<mat-menu #downloadMenu="matMenu"> | ||
<button mat-menu-item aria-label="download" (click)="onDownloadSheetGSTR('main')"> | ||
{{ localeData?.filing?.download_sheet }} | ||
</button> | ||
<button mat-menu-item aria-label="download" (click)="onDownloadSheetGSTR('error')"> | ||
{{ localeData?.filing?.download_error_sheet }} | ||
</button> | ||
</mat-menu> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Leverage code reusability for multiple GSTR sections
Similar logic exists in GSTR1 and GSTR2 sections for downloading sheets, error sheets, etc. Evaluate extracting this code into a shared component or service to adhere to DRY (Don't Repeat Yourself) principles.
import { MatDialogModule } from '@angular/material/dialog'; | ||
import { MatFormFieldModule } from '@angular/material/form-field'; | ||
import { FormFieldsModule } from '../../theme/form-fields/form-fields.module'; | ||
import { MatButtonModule } from '@angular/material/button'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding MatInputModule for complete Material form field support.
While you've added MatFormFieldModule, the Material form fields typically require MatInputModule to work properly with input elements.
Add the following import:
import { MatDialogModule } from '@angular/material/dialog';
import { MatFormFieldModule } from '@angular/material/form-field';
import { FormFieldsModule } from '../../theme/form-fields/form-fields.module';
import { MatButtonModule } from '@angular/material/button';
+ import { MatInputModule } from '@angular/material/input';
📝 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 { MatDialogModule } from '@angular/material/dialog'; | |
import { MatFormFieldModule } from '@angular/material/form-field'; | |
import { FormFieldsModule } from '../../theme/form-fields/form-fields.module'; | |
import { MatButtonModule } from '@angular/material/button'; | |
import { MatDialogModule } from '@angular/material/dialog'; | |
import { MatFormFieldModule } from '@angular/material/form-field'; | |
import { FormFieldsModule } from '../../theme/form-fields/form-fields.module'; | |
import { MatButtonModule } from '@angular/material/button'; | |
import { MatInputModule } from '@angular/material/input'; |
@@ -533,7 +533,7 @@ export class UpdateEwayVehicle { | |||
public reasonRem: string; | |||
|
|||
public transDocNo?: string; | |||
public transDocDate?: string; | |||
public transDocDate?: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid using any
type for transDocDate
Using any
type reduces type safety. Since this is a document date field, consider using a more specific type.
- public transDocDate?: any;
+ public transDocDate?: string | Date;
📝 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.
public transDocDate?: any; | |
public transDocDate?: string | Date; |
public getSearchFieldText(fieldName: string): string { | ||
if (fieldName === "name") { | ||
return this.localeData?.search_name; | ||
} | ||
return ""; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add null check for localeData access
The method might return undefined if localeData
is not initialized. Consider adding a null check:
public getSearchFieldText(fieldName: string): string {
if (fieldName === "name") {
- return this.localeData?.search_name;
+ return this.localeData?.search_name ?? "Search by name";
}
return "";
}
📝 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.
public getSearchFieldText(fieldName: string): string { | |
if (fieldName === "name") { | |
return this.localeData?.search_name; | |
} | |
return ""; | |
} | |
public getSearchFieldText(fieldName: string): string { | |
if (fieldName === "name") { | |
return this.localeData?.search_name ?? "Search by name"; | |
} | |
return ""; | |
} |
import { Router } from '@angular/router'; | ||
import { UntypedFormControl } from "@angular/forms"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using typed FormControl instead of UntypedFormControl
UntypedFormControl
is deprecated in favor of the typed FormControl
. Consider updating to:
-import { UntypedFormControl } from "@angular/forms";
+import { FormControl } from "@angular/forms";
📝 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 { UntypedFormControl } from "@angular/forms"; | |
import { FormControl } from "@angular/forms"; |
public deleteTax(taxToDelete): void { | ||
// this.newTaxObj = taxToDelete; | ||
// this.selectedTax = this.availableTaxes.find((tax) => tax?.uniqueName === taxToDelete?.uniqueName); | ||
// let message = this.localeData?.tax_delete_message; | ||
// message = message?.replace("[TAX_NAME]", this.selectedTax.name); | ||
// this.confirmationMessage = message; | ||
// this.confirmationFor = 'delete'; | ||
// this.taxConfirmationModel?.show(); | ||
|
||
this.dialog.open(this.taxConfirmationModel, { | ||
width: '580px' | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Simplify dialog implementation
The implementation has both old and new code, making it harder to maintain.
Consider cleaning up the implementation:
public deleteTax(taxToDelete): void {
- // this.newTaxObj = taxToDelete;
- // this.selectedTax = this.availableTaxes.find((tax) => tax?.uniqueName === taxToDelete?.uniqueName);
- // let message = this.localeData?.tax_delete_message;
- // message = message?.replace("[TAX_NAME]", this.selectedTax.name);
- // this.confirmationMessage = message;
- // this.confirmationFor = 'delete';
- // this.taxConfirmationModel?.show();
this.newTaxObj = taxToDelete;
this.selectedTax = this.availableTaxes.find((tax) => tax?.uniqueName === taxToDelete?.uniqueName);
this.dialog.open(this.taxConfirmationModel, {
width: '580px'
});
}
📝 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.
public deleteTax(taxToDelete): void { | |
// this.newTaxObj = taxToDelete; | |
// this.selectedTax = this.availableTaxes.find((tax) => tax?.uniqueName === taxToDelete?.uniqueName); | |
// let message = this.localeData?.tax_delete_message; | |
// message = message?.replace("[TAX_NAME]", this.selectedTax.name); | |
// this.confirmationMessage = message; | |
// this.confirmationFor = 'delete'; | |
// this.taxConfirmationModel?.show(); | |
this.dialog.open(this.taxConfirmationModel, { | |
width: '580px' | |
}); | |
public deleteTax(taxToDelete): void { | |
this.newTaxObj = taxToDelete; | |
this.selectedTax = this.availableTaxes.find((tax) => tax?.uniqueName === taxToDelete?.uniqueName); | |
this.dialog.open(this.taxConfirmationModel, { | |
width: '580px' | |
}); |
/** Custom selected month */ | ||
public customMonth: string = ''; | ||
/** Holds start month/year */ | ||
public startAt: Date = new Date(); | ||
/** Holds selected date */ | ||
public date: FormControl = new FormControl(''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Initialize FormControl with a default value.
The FormControl is initialized without a default value, which could lead to undefined behavior.
- public date: FormControl = new FormControl('');
+ public date: FormControl = new FormControl(dayjs().format(GIDDH_DATE_FORMAT_MONTH_YEAR));
Committable suggestion skipped: line range outside the PR's diff.
public setMonthAndYear(date: any, datepicker: MatDatepicker<dayjs.Dayjs>): void { | ||
datepicker.close(); | ||
let selectedMonth = new Date(date); | ||
let firstDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth(), 1); | ||
let lastDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth() + 1, 0); | ||
this.dateSelected([firstDay, lastDay]); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling to setMonthAndYear method.
The method should handle potential null/undefined values for the date parameter.
- public setMonthAndYear(date: any, datepicker: MatDatepicker<dayjs.Dayjs>): void {
+ public setMonthAndYear(date: Date | null, datepicker: MatDatepicker<dayjs.Dayjs>): void {
+ if (!date) {
+ return;
+ }
datepicker.close();
- let selectedMonth = new Date(date);
- let firstDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth(), 1);
- let lastDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth() + 1, 0);
+ const selectedMonth = new Date(date);
+ const firstDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth(), 1);
+ const lastDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth() + 1, 0);
this.dateSelected([firstDay, lastDay]);
}
📝 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.
public setMonthAndYear(date: any, datepicker: MatDatepicker<dayjs.Dayjs>): void { | |
datepicker.close(); | |
let selectedMonth = new Date(date); | |
let firstDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth(), 1); | |
let lastDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth() + 1, 0); | |
this.dateSelected([firstDay, lastDay]); | |
} | |
public setMonthAndYear(date: Date | null, datepicker: MatDatepicker<dayjs.Dayjs>): void { | |
if (!date) { | |
return; | |
} | |
datepicker.close(); | |
const selectedMonth = new Date(date); | |
const firstDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth(), 1); | |
const lastDay = new Date(selectedMonth.getFullYear(), selectedMonth.getMonth() + 1, 0); | |
this.dateSelected([firstDay, lastDay]); | |
} |
public dateSelected(event: any): void { | ||
this.customMonth = event[0].toLocaleString('en-us', { month: 'long', year: 'numeric' }); | ||
this.date.setValue(this.customMonth); | ||
this.periodChanged({ from: event[0], to: event[1] }); | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add type safety to dateSelected method.
The event parameter should be properly typed for better type safety.
- public dateSelected(event: any): void {
+ public dateSelected(event: Date[]): void {
this.customMonth = event[0].toLocaleString('en-us', { month: 'long', year: 'numeric' });
this.date.setValue(this.customMonth);
this.periodChanged({ from: event[0], to: event[1] });
}
📝 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.
public dateSelected(event: any): void { | |
this.customMonth = event[0].toLocaleString('en-us', { month: 'long', year: 'numeric' }); | |
this.date.setValue(this.customMonth); | |
this.periodChanged({ from: event[0], to: event[1] }); | |
} | |
public dateSelected(event: Date[]): void { | |
this.customMonth = event[0].toLocaleString('en-us', { month: 'long', year: 'numeric' }); | |
this.date.setValue(this.customMonth); | |
this.periodChanged({ from: event[0], to: event[1] }); | |
} |
<div class="form-group col-sm-6 col-xs-12"> | ||
<label>{{ commonLocaleData?.app_tax_authority }} <span class="text-danger">*</span> </label> | ||
<br /> | ||
<sh-select | ||
[options]="taxAuthorityList" | ||
name="taxAuthority" | ||
[(ngModel)]="newTaxObj.taxAuthorityRequest.uniqueName" | ||
[defaultValue]="selectedTaxAuthority" | ||
[placeholder]="commonLocaleData?.app_select + ' ' + commonLocaleData?.app_tax_authority" | ||
[ngClass]="{ 'placeholder-black': selectedTaxAuthority }" | ||
[ItemHeight]="33" | ||
required | ||
[isRequired]="true" | ||
></sh-select> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate form control.
There are two implementations of the same form control (dropdown-field and sh-select) for tax authority selection.
Keep only the Material Design version (dropdown-field) and remove the legacy implementation (sh-select):
- <div class="form-group col-sm-6 col-xs-12">
- <label>{{ commonLocaleData?.app_tax_authority }} <span class="text-danger">*</span> </label>
- <br />
- <sh-select
- [options]="taxAuthorityList"
- name="taxAuthority"
- [(ngModel)]="newTaxObj.taxAuthorityRequest.uniqueName"
- [defaultValue]="selectedTaxAuthority"
- [placeholder]="commonLocaleData?.app_select + ' ' + commonLocaleData?.app_tax_authority"
- [ngClass]="{ 'placeholder-black': selectedTaxAuthority }"
- [ItemHeight]="33"
- required
- [isRequired]="true"
- ></sh-select>
- </div>
Committable suggestion skipped: line range outside the PR's diff.
</pagination> | ||
<div class="d-flex justify-content-end" *ngIf="pagination.totalPages > 1 && !isLoading"> | ||
<mat-paginator | ||
class="mt-3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove space
@@ -69,6 +60,17 @@ export class FilingStatusComponent implements OnInit, OnDestroy { | |||
public startAt: Date = new Date(); | |||
/** Holds selected date */ | |||
public date: FormControl = new FormControl(); | |||
/** Holds page Size Options for pagination */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/** Holds page Size Options for pagination */ | |
/** Holds page size options for pagination */ |
"page": 1, | ||
"count": this.pageSizeOptions[0], | ||
"totalPages": 1, | ||
"totalItems": 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"totalItems": 1, | |
"totalItems": 1 |
public pageSizeOptions: any[] = PAGE_SIZE_OPTIONS; | ||
/** Hold table page index number */ | ||
public pageIndex: number = 0; | ||
/** Holds Pagination Data */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/** Holds Pagination Data */ | |
/** Holds pagination data */ |
if (typeof event?.pageIndex === 'number') { | ||
this.pageIndex = event.pageIndex; | ||
this.pagination.page = event.pageIndex + 1; | ||
this.pagination.count = event.pageSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.pagination.count = event.pageSize; | |
this.pagination.count = event?.pageSize; |
</th> | ||
<td mat-cell *matCellDef="let element"> | ||
<span class="d-inline-flex"> | ||
<amount-field [amount]="element.taxRate" [currencySymbol]="false" [currencyCode]="false"></amount-field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check currency symbol and code in amount field
</amount-field> | ||
</span> | ||
<div *ngIf="inlineSearch === 'invoiceNumber'"> | ||
<input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove commented code in over all file
<th mat-header-cell *matHeaderCellDef mat-sort-header (click)="sortReverseChargeList('entryDate')"> | ||
{{ localeData?.entry_date }} | ||
</th> | ||
<td mat-cell *matCellDef="let element">{{ element.entryDate }}</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<td mat-cell *matCellDef="let element">{{ element.entryDate }}</td> | |
<td mat-cell *matCellDef="let element">{{ element?.entryDate }}</td> |
Correct in over all file
<ng-container matColumnDef="invoiceNumber"> | ||
<th mat-header-cell *matHeaderCellDef> {{ localeData?.filing?.invoice_no }} </th> | ||
<td mat-cell *matCellDef="let item"> | ||
<span [ngClass]="{'btn-link': item.voucherType !== 'purchase' }" (click)="onSelectInvoice(item)"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use variable of enum here and check in over all file
<i class="icon-rupees icon-font-11"></i> | ||
<span class="d-inline-flex"> | ||
<amount-field | ||
[amount]="element.igstAmount" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use currency and symbol if there
https://app.clickup.com/t/86cwmfhbm
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Documentation
Chores