-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Download qwd file UI changes #1059
feat: Download qwd file UI changes #1059
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces significant updates to the integration configuration for QuickBooks Desktop across multiple files. Key changes include renaming the NetSuite configuration to QuickBooks Desktop, updating instructional texts, and modifying component structures to enhance user interaction. The Changes
Possibly related PRs
Warning Rate limit exceeded@github-actions[bot] has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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 (
|
@@ -0,0 +1,3 @@ | |||
<svg width="24" height="20" viewBox="0 0 24 20" fill="none" xmlns="http://www.w3.org/2000/svg"> |
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.
do similar to onboarding stepper, remove this file
|
…-onboarding-download-file-UI
* feat: qbd connector setup UI changes * Merge branch qbd-direct-onboarding-download-file-UI into qbd-direct-step-connector-UI * feat: Qbd direct connection data sync UI changes (#1061) * feat: Qbd direct connection data sync UI changes * input made required * svg update * feat: qbd direct pre requisite ts changes (#1062) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes (#1063) * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes (#1064) * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes (#1065) * PR comment fix * PR comment fix * Qbd direct connector data sync up ts (#1070) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes * feat: QBD direct main connection page business logic (#1066) * feat: QBD direct main connection page business logic * onboarding connection ts changes * onboarding connection ts changes --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]>
fd801b5
into
qbd-direct-onboarding-pre-requisite-UI
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: 8
🧹 Outside diff range and nitpick comments (10)
src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.html (1)
1-1
: Consider enhancing component integration with available propertiesThe component could benefit from utilizing its available input properties to better control its behavior:
-<app-qbd-direct-download-file></app-qbd-direct-download-file> +<div class="qbd-download-container"> + <app-qbd-direct-download-file + [isLoading]="loading" + [showDownloadLink]="canShowDownload" + [isStepCompleted]="isCurrentStepCompleted" + [isCompanyPathInvalid]="hasInvalidPath" + (stepComplete)="onStepComplete()"> + </app-qbd-direct-download-file> +</div>This would:
- Provide better control over the component's state and behavior
- Add proper structural elements for layout management
- Handle step completion events from the child component
src/app/integrations/qbd-direct/qbd-direct.module.ts (1)
Line range hint
5-5
: Remove unused import and fix component declarationThe
QbdDirectComponent
is imported but not used. Additionally, thedeclarations
array is empty which is suspicious. If the component is needed, it should be in thedeclarations
array, not inimports
.Apply this diff to fix the issues:
- import { QbdDirectComponent } from './qbd-direct.component'; @NgModule({ - declarations: [], + declarations: [QbdDirectComponent], imports: [ CommonModule, QbdDirectRoutingModule, SharedModule ] })If the component is not needed at all, remove the import line entirely.
Also applies to: 7-7
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.scss (2)
1-19
: Consider alternative approaches to reduce !important usageWhile the
!important
flags might be necessary to override PrimeNG's default styles, extensive use can make the styles harder to maintain. Consider these alternatives:
- Use more specific selectors
- Increase selector specificity through parent classes
- Configure PrimeNG theme variables where possible
Example of increasing specificity:
-.downloadBtn, .downloadBtn:hover { +.qbd-download-container .downloadBtn, +.qbd-download-container .downloadBtn:hover { @apply tw-h-40-px tw-text-14-px tw-font-500 tw-text-white tw-bg-btn-secondary-bg tw-rounded-sm; }
5-7
: Consider documenting ::ng-deep usageWhile ::ng-deep is necessary for styling PrimeNG components, it's technically deprecated. Consider adding a comment explaining why it's needed here, to prevent future developers from removing it without understanding the implications.
Add documentation like:
// ::ng-deep is required here to style PrimeNG internal elements // TODO: Update once Angular provides an official alternative to ::ng-deepAlso applies to: 9-11, 13-15, 17-19
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.ts (1)
19-25
: Add type annotations and documentation for input propertiesWhile the property names are descriptive, adding TypeScript type annotations and JSDoc comments would improve code maintainability and developer experience.
Consider applying this improvement:
- @Input({required: true}) isLoading: boolean; + /** + * Flag indicating if the component is in loading state + */ + @Input({required: true}) isLoading!: boolean; - @Input({required: true}) showDownloadLink: boolean; + /** + * Flag controlling the visibility of the download link + */ + @Input({required: true}) showDownloadLink!: boolean; - @Input({required: true}) isStepCompleted: boolean; + /** + * Flag indicating if the current step is completed + */ + @Input({required: true}) isStepCompleted!: boolean; - @Input({required: true}) isCompanyPathInvalid: boolean; + /** + * Flag indicating if the company path is invalid + */ + @Input({required: true}) isCompanyPathInvalid!: boolean;src/app/shared/components/configuration/configuration-step-footer/configuration-step-footer.component.html (2)
1-1
: Consider simplifying complex template conditionsThe template contains complex conditional logic that could be moved to the component class for better maintainability.
Consider refactoring the template logic:
-<div class="tw-flex tw-py-16-px tw-px-24-px" [ngClass]="[!showBackButton ? 'tw-justify-end': 'tw-justify-between', ctaText === 'Next' ? '' : 'tw-border-t tw-border-t-border-tertiary']"> +<div class="tw-flex tw-py-16-px tw-px-24-px" [ngClass]="getFooterClasses()">And in the component class:
getFooterClasses(): string[] { return [ this.showBackButton ? 'tw-justify-between' : 'tw-justify-end', this.ctaText === 'Next' ? '' : 'tw-border-t tw-border-t-border-tertiary' ]; }
Line range hint
1-31
: Consider architectural improvements for better maintainabilityThe component handles multiple concerns (branding, loading states, navigation) which could be better organized.
Suggestions for improvement:
- Create a dedicated loading button component to handle loading states consistently:
@Component({ selector: 'app-loading-button', template: ` <button pButton [disabled]="isLoading" ...> {{ text }} <app-loading-spinner *ngIf="isLoading" /> </button> ` })
- Move brand-specific text transformations to a service:
@Injectable() class BrandingService { formatButtonText(text: string, brandId: string): string { return brandId === 'co' ? sentenceCase(text) : text; } }These changes would make the component more maintainable and easier to test.
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.html (3)
1-17
: Enhance accessibility for step indicator and success stateThe header structure is clean, but needs accessibility improvements for screen readers.
Apply these changes:
-<div class="tw-w-20-px tw-h-20-px tw-rounded-sm tw-bg-bg-secondary tw-text-white tw-text-14-px tw-font-500 tw-text-center tw-mr-8-px"> +<div class="tw-w-20-px tw-h-20-px tw-rounded-sm tw-bg-bg-secondary tw-text-white tw-text-14-px tw-font-500 tw-text-center tw-mr-8-px" role="progressbar" aria-label="Step 1 of configuration"> 1 </div> ... -<div class="tw-w-24-px tw-h-24-px tw-bg-success-toast tw-rounded-full tw-flex tw-items-center tw-justify-center"> +<div class="tw-w-24-px tw-h-24-px tw-bg-success-toast tw-rounded-full tw-flex tw-items-center tw-justify-center" aria-label="Step completed">
23-37
: Improve input field accessibilityThe file path input field needs better accessibility attributes for screen readers.
Enhance the input field:
<div class="tw-pt-16-px"> + <label for="downloadFilePath" class="tw-sr-only">Company file path</label> <input type="text" pInputText id="downloadFilePath" name="downloadFilePath" [(ngModel)]="downloadFilePath" placeholder="Example: C:\Users\[Username]\Documents\QuickBooks" class="tw-w-full tw-border tw-border-input-default-border tw-rounded-sm tw-px-8-px tw-py-12-px" required pattern="^(.+[\/\\]).*$" #downloadFilePathField="ngModel" + aria-describedby="pathHelpText pathError" [ngClass]="{ 'error-box': downloadFilePathField.invalid && downloadFilePathField.touched }"> - <div *ngIf="downloadFilePathField.invalid && downloadFilePathField.touched" class="tw-text-alert-toast"> + <div *ngIf="downloadFilePathField.invalid && downloadFilePathField.touched" class="tw-text-alert-toast" id="pathError" role="alert"> <small *ngIf="downloadFilePathField.errors?.required || downloadFilePathField.errors?.pattern" class="tw-flex"> <app-svg-icon [svgSource]="'info-circle-fill'" [isTextColorAllowed]="true" [width]="'16px'" [height]="'16px'" [styleClasses]="'tw-text-form-error-help-text-color'"></app-svg-icon> Enter a valid company file path. </small> </div> + <div class="tw-sr-only" id="pathHelpText">Enter the complete path to your QuickBooks company file</div> </div>
54-57
: Add loading state accessibility attributesThe loading state needs ARIA attributes for better screen reader support.
Enhance the loading state:
-<div *ngIf="isLoading" class="tw-flex tw-flex-col tw-justify-center tw-items-center tw-h-[400px]"> +<div *ngIf="isLoading" class="tw-flex tw-flex-col tw-justify-center tw-items-center tw-h-[400px]" role="status" aria-live="polite"> <app-loader styleClass="spinner-30 tw-top-2-px"></app-loader> - <span class="tw-text-14-px tw-font-400 tw-text-text-primary">Loading</span> + <span class="tw-text-14-px tw-font-400 tw-text-text-primary">Loading integration file generator</span> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
src/assets/sprites/sprite.svg
is excluded by!**/*.svg
📒 Files selected for processing (11)
src/app/branding/c1-contents-config.ts
(1 hunks)src/app/branding/fyle-contents-config.ts
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.html
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.ts
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.html
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.scss
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.ts
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-shared.module.ts
(1 hunks)src/app/integrations/qbd-direct/qbd-direct.module.ts
(1 hunks)src/app/shared/components/configuration/configuration-step-footer/configuration-step-footer.component.html
(2 hunks)src/styles.scss
(1 hunks)
🔇 Additional comments (11)
src/app/integrations/qbd-direct/qbd-direct.module.ts (1)
Line range hint 3-3
: Verify component usage in routing
Let's verify if QbdDirectComponent
is actually used in the routing module to determine if it should be declared here.
✅ Verification successful
Component is used in routing and should be declared in the module
The verification confirms that QbdDirectComponent
is actively used in the routing configuration (qbd-direct-routing.module.ts
) as a route component. It's set as the component for a parent route that contains child routes. Therefore, it needs to be declared in the QbdDirectModule
.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if QbdDirectComponent is used in routing
# Expected: Find route configurations using QbdDirectComponent
# Check routing module for component usage
rg -A 5 "QbdDirectComponent" "src/app/integrations/qbd-direct/qbd-direct-routing.module.ts"
# Also check if component is used elsewhere
ast-grep --pattern 'QbdDirectComponent'
Length of output: 1189
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-shared.module.ts (1)
13-15
: LGTM: Component export is correctly configured
The component is properly exported, making it available for use in other modules that import QbdDirectSharedModule.
src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.ts (2)
2-3
: LGTM! Import statements are properly structured.
The imports follow Angular's best practices with relative paths for feature modules and absolute paths for shared modules.
8-8
: Verify the usage of imported modules in the template.
While the imports are correctly added, let's verify their usage in the template file.
Additionally, since this component is part of a file download feature:
Consider implementing the following improvements:
- Add error handling methods for failed downloads
- Include loading state management
- Add download progress tracking if applicable
Would you like me to provide example implementations for these improvements?
✅ Verification successful
Let me check the actual content of the template file to verify how these modules are being used.
Both imported modules are correctly used in the component
The template uses <app-qbd-direct-download-file>
component which comes from QbdDirectSharedModule
, and the SharedModule
is likely needed for common Angular features in the component class. The imports are justified.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of imported modules in the template file
# Check for components/directives from QbdDirectSharedModule
echo "Checking for QbdDirectSharedModule components usage:"
rg -l "app-qbd-direct-" "src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.html"
# Check for components/directives from SharedModule
echo "Checking for SharedModule components/directives usage:"
rg -l "\\*ng[A-Z]|\\[(ng[A-Z])|<app-" "src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.html"
Length of output: 827
Script:
#!/bin/bash
# Get the actual content of the template file to see how the modules are used
cat "src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.html"
Length of output: 200
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.scss (1)
1-19
: LGTM on Tailwind usage!
The consistent use of Tailwind utility classes through @apply
directives maintains a clean and maintainable styling approach. The chosen utilities align well with the design system (colors, spacing, typography).
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.ts (2)
27-29
:
Initialize and type the downloadFilePath property
The downloadFilePath
property is declared but not initialized, which could lead to undefined behavior.
Consider applying this fix:
- downloadFilePath: string;
+ downloadFilePath: string = '';
Also, verify where this property is being set and used.
#!/bin/bash
# Description: Check usage of downloadFilePath in component files
# Search for downloadFilePath usage in component files
rg "downloadFilePath" src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/
1-8
: Verify usage of imported modules and validators
The required
import from '@rxweb/reactive-form-validators' appears to be unused as there's no form implementation visible in the component.
Let's verify the usage of imports:
Also applies to: 13-13
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.html (1)
69-71
: LGTM! Footer implementation is clean and reusable
The footer section properly uses a shared configuration step component with appropriate button state management.
src/styles.scss (1)
493-495
: LGTM! The spinner implementation follows established patterns.
The new .spinner-30
class is well-implemented using the existing spinner
mixin, maintaining consistency with other spinner variants while providing an intermediate size option.
src/app/branding/c1-contents-config.ts (1)
19-20
: LGTM! Text updates for QuickBooks Desktop integration.
The configuration text changes accurately reflect the QuickBooks Desktop integration context.
src/app/branding/fyle-contents-config.ts (1)
19-20
: LGTM! Text changes align with QuickBooks Desktop integration.
The configuration text has been correctly updated to reference QuickBooks Desktop instead of NetSuite.
SharedModule, | ||
QbdDirectDownloadFileComponent | ||
], |
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.
Critical: Incorrect module configuration for QbdDirectDownloadFileComponent
Components should be declared in the declarations
array, not the imports
array. The imports
array is specifically for importing other modules.
Apply this fix:
@NgModule({
- declarations: [],
+ declarations: [
+ QbdDirectDownloadFileComponent
+ ],
imports: [
CommonModule,
SharedModule,
- QbdDirectDownloadFileComponent
],
exports: [
QbdDirectDownloadFileComponent
]
})
📝 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.
SharedModule, | |
QbdDirectDownloadFileComponent | |
], | |
SharedModule | |
], | |
declarations: [ | |
QbdDirectDownloadFileComponent | |
], | |
exports: [ | |
QbdDirectDownloadFileComponent | |
] |
continueToNextStep() { | ||
} | ||
|
||
onDownloadClick() { | ||
// Emit output | ||
} | ||
|
||
onManualDownload() { | ||
// Emit output | ||
} | ||
|
||
onRetryClick() { | ||
// Emit output | ||
} |
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.
Implement method logic and add event emitters
The methods are currently empty with comments indicating they should emit outputs, but no EventEmitter properties are defined.
Consider applying these changes:
+ @Output() private readonly download = new EventEmitter<void>();
+ @Output() private readonly manualDownload = new EventEmitter<void>();
+ @Output() private readonly retry = new EventEmitter<void>();
+ @Output() private readonly continue = new EventEmitter<void>();
- continueToNextStep() {
+ continueToNextStep(): void {
+ this.continue.emit();
}
- onDownloadClick() {
- // Emit output
+ onDownloadClick(): void {
+ this.download.emit();
}
- onManualDownload() {
- // Emit output
+ onManualDownload(): void {
+ this.manualDownload.emit();
}
- onRetryClick() {
- // Emit output
+ onRetryClick(): void {
+ this.retry.emit();
}
Don't forget to import Output and EventEmitter:
-import { Component, Input } from '@angular/core';
+import { Component, Input, Output, EventEmitter } from '@angular/core';
📝 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.
continueToNextStep() { | |
} | |
onDownloadClick() { | |
// Emit output | |
} | |
onManualDownload() { | |
// Emit output | |
} | |
onRetryClick() { | |
// Emit output | |
} | |
@Output() private readonly download = new EventEmitter<void>(); | |
@Output() private readonly manualDownload = new EventEmitter<void>(); | |
@Output() private readonly retry = new EventEmitter<void>(); | |
@Output() private readonly continue = new EventEmitter<void>(); | |
continueToNextStep(): void { | |
this.continue.emit(); | |
} | |
onDownloadClick(): void { | |
this.download.emit(); | |
} | |
onManualDownload(): void { | |
this.manualDownload.emit(); | |
} | |
onRetryClick(): void { | |
this.retry.emit(); | |
} |
@@ -21,7 +21,7 @@ | |||
<span *ngIf="brandingConfig.brandId === 'fyle'"> | |||
{{ ctaText }} | |||
</span> | |||
<app-svg-icon *ngIf="brandingFeatureConfig.isIconsInsideButtonAllowed && ctaText !== 'Save'" [svgSource]="'arrow-tail-right-medium'" [width]="'18px'" [height]="'18px'" [styleClasses]="'tw-text-white tw-pl-10-px tw-pt-2-px tw-text-12-px'"></app-svg-icon> | |||
<app-svg-icon *ngIf="brandingFeatureConfig.isIconsInsideButtonAllowed && ctaText !== 'Save' && ctaText !== 'Next'" [svgSource]="'arrow-tail-right-medium'" [width]="'18px'" [height]="'18px'" [styleClasses]="'tw-text-white tw-pl-10-px tw-pt-2-px tw-text-12-px'"></app-svg-icon> |
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
Improve icon visibility logic and styling
The icon visibility condition is becoming complex and relies on specific text values, which could make maintenance difficult.
Consider these improvements:
- Move the visibility logic to the component:
-<app-svg-icon *ngIf="brandingFeatureConfig.isIconsInsideButtonAllowed && ctaText !== 'Save' && ctaText !== 'Next'"
+<app-svg-icon *ngIf="shouldShowIcon()"
- Create an enum or constant for the button states:
enum ButtonState {
SAVE = 'Save',
NEXT = 'Next',
// ... other states
}
shouldShowIcon(): boolean {
return this.brandingFeatureConfig.isIconsInsideButtonAllowed &&
![ButtonState.SAVE, ButtonState.NEXT].includes(this.ctaText);
}
- Consider moving the styling classes to a CSS class:
-[styleClasses]="'tw-text-white tw-pl-10-px tw-pt-2-px tw-text-12-px'"
+class="button-icon"
<div class="tw-rounded-border-radius-2xl"> | ||
<p-card> | ||
<div class="tw-flex tw-justify-center"> | ||
<iframe width="427" height="259" src="https://www.youtube.com/embed/2oYdc8KcQnk" frameborder="0"></iframe> |
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.
Enhance YouTube iframe security and accessibility
The iframe needs security attributes and accessibility improvements.
Apply these security and accessibility enhancements:
-<iframe width="427" height="259" src="https://www.youtube.com/embed/2oYdc8KcQnk" frameborder="0"></iframe>
+<iframe
+ width="427"
+ height="259"
+ src="https://www.youtube.com/embed/2oYdc8KcQnk"
+ title="Guide: How to find QuickBooks company file path"
+ allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
+ sandbox="allow-scripts allow-same-origin allow-presentation"
+ loading="lazy"
+></iframe>
📝 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.
<iframe width="427" height="259" src="https://www.youtube.com/embed/2oYdc8KcQnk" frameborder="0"></iframe> | |
<iframe | |
width="427" | |
height="259" | |
src="https://www.youtube.com/embed/2oYdc8KcQnk" | |
title="Guide: How to find QuickBooks company file path" | |
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" | |
sandbox="allow-scripts allow-same-origin allow-presentation" | |
loading="lazy" | |
></iframe> |
<input type="text" pInputText id="downloadFilePath" name="downloadFilePath" [(ngModel)]="downloadFilePath" | ||
placeholder="Example: C:\Users\[Username]\Documents\QuickBooks" | ||
class="tw-w-full tw-border tw-border-input-default-border tw-rounded-sm tw-px-8-px tw-py-12-px" | ||
required pattern="^(.+)\/([^\/]+)$" | ||
#downloadFilePathField="ngModel" | ||
[ngClass]="{ | ||
'error-box': downloadFilePathField.invalid && downloadFilePathField.touched | ||
}"> |
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.
Fix path validation pattern mismatch
The validation pattern uses forward slashes (^(.+)\/([^\/]+)$
) but the placeholder example shows Windows-style backslashes. This could confuse users and cause valid paths to fail validation.
Update the pattern to support both slash types:
-required pattern="^(.+)\/([^\/]+)$"
+required pattern="^(.+[\/\\]).*$"
📝 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.
<input type="text" pInputText id="downloadFilePath" name="downloadFilePath" [(ngModel)]="downloadFilePath" | |
placeholder="Example: C:\Users\[Username]\Documents\QuickBooks" | |
class="tw-w-full tw-border tw-border-input-default-border tw-rounded-sm tw-px-8-px tw-py-12-px" | |
required pattern="^(.+)\/([^\/]+)$" | |
#downloadFilePathField="ngModel" | |
[ngClass]="{ | |
'error-box': downloadFilePathField.invalid && downloadFilePathField.touched | |
}"> | |
<input type="text" pInputText id="downloadFilePath" name="downloadFilePath" [(ngModel)]="downloadFilePath" | |
placeholder="Example: C:\Users\[Username]\Documents\QuickBooks" | |
class="tw-w-full tw-border tw-border-input-default-border tw-rounded-sm tw-px-8-px tw-py-12-px" | |
required pattern="^(.+[\/\\]).*$" | |
#downloadFilePathField="ngModel" | |
[ngClass]="{ | |
'error-box': downloadFilePathField.invalid && downloadFilePathField.touched | |
}"> |
<div *ngIf="!isLoading && showDownloadLink" class="tw-flex tw-flex-col tw-justify-center tw-items-center tw-h-[400px] tw-text-center"> | ||
<div class="tw-text-center"> | ||
<app-svg-icon svgSource="check-circle-outline" [width]="'24px'" [height]="'24px'" [isTextColorAllowed]="true" [styleClasses]="'tw-text-success-toast tw-pr-6-px'"></app-svg-icon> | ||
</div> | ||
<div> | ||
<p class="tw-pt-4-px tw-text-text-tertiary tw-text-16-px tw-font-500">Your download will begin</p> | ||
<span class="tw-pt-8-px tw-text-text-muted tw-text-12-px tw-font-400">If it didn’t start, <a class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onManualDownload()">download the integration file manually</a>.</span> | ||
<p class="tw-pt-16-px tw-text-text-muted tw-text-12-px tw-font-400">Not the right file? <a class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onRetryClick()">Try again.</a></p> | ||
</div> | ||
</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.
🛠️ Refactor suggestion
Enhance download success state accessibility and error handling
The success state needs better accessibility and clearer error handling for failed downloads.
Apply these improvements:
-<div *ngIf="!isLoading && showDownloadLink" class="tw-flex tw-flex-col tw-justify-center tw-items-center tw-h-[400px] tw-text-center">
+<div *ngIf="!isLoading && showDownloadLink" class="tw-flex tw-flex-col tw-justify-center tw-items-center tw-h-[400px] tw-text-center" role="alert" aria-live="polite">
<div class="tw-text-center">
- <app-svg-icon svgSource="check-circle-outline" [width]="'24px'" [height]="'24px'" [isTextColorAllowed]="true" [styleClasses]="'tw-text-success-toast tw-pr-6-px'"></app-svg-icon>
+ <app-svg-icon svgSource="check-circle-outline" [width]="'24px'" [height]="'24px'" [isTextColorAllowed]="true" [styleClasses]="'tw-text-success-toast tw-pr-6-px'" aria-hidden="true"></app-svg-icon>
</div>
<div>
<p class="tw-pt-4-px tw-text-text-tertiary tw-text-16-px tw-font-500">Your download will begin</p>
- <span class="tw-pt-8-px tw-text-text-muted tw-text-12-px tw-font-400">If it didn't start, <a class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onManualDownload()">download the integration file manually</a>.</span>
+ <span class="tw-pt-8-px tw-text-text-muted tw-text-12-px tw-font-400">If it didn't start, <button class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onManualDownload()" aria-label="Download integration file manually">download the integration file manually</button>.</span>
- <p class="tw-pt-16-px tw-text-text-muted tw-text-12-px tw-font-400">Not the right file? <a class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onRetryClick()">Try again.</a></p>
+ <p class="tw-pt-16-px tw-text-text-muted tw-text-12-px tw-font-400">Not the right file? <button class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onRetryClick()" aria-label="Retry with different file path">Try again</button>.</p>
</div>
</div>
📝 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.
<div *ngIf="!isLoading && showDownloadLink" class="tw-flex tw-flex-col tw-justify-center tw-items-center tw-h-[400px] tw-text-center"> | |
<div class="tw-text-center"> | |
<app-svg-icon svgSource="check-circle-outline" [width]="'24px'" [height]="'24px'" [isTextColorAllowed]="true" [styleClasses]="'tw-text-success-toast tw-pr-6-px'"></app-svg-icon> | |
</div> | |
<div> | |
<p class="tw-pt-4-px tw-text-text-tertiary tw-text-16-px tw-font-500">Your download will begin</p> | |
<span class="tw-pt-8-px tw-text-text-muted tw-text-12-px tw-font-400">If it didn’t start, <a class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onManualDownload()">download the integration file manually</a>.</span> | |
<p class="tw-pt-16-px tw-text-text-muted tw-text-12-px tw-font-400">Not the right file? <a class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onRetryClick()">Try again.</a></p> | |
</div> | |
</div> | |
<div *ngIf="!isLoading && showDownloadLink" class="tw-flex tw-flex-col tw-justify-center tw-items-center tw-h-[400px] tw-text-center" role="alert" aria-live="polite"> | |
<div class="tw-text-center"> | |
<app-svg-icon svgSource="check-circle-outline" [width]="'24px'" [height]="'24px'" [isTextColorAllowed]="true" [styleClasses]="'tw-text-success-toast tw-pr-6-px'" aria-hidden="true"></app-svg-icon> | |
</div> | |
<div> | |
<p class="tw-pt-4-px tw-text-text-tertiary tw-text-16-px tw-font-500">Your download will begin</p> | |
<span class="tw-pt-8-px tw-text-text-muted tw-text-12-px tw-font-400">If it didn't start, <button class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onManualDownload()" aria-label="Download integration file manually">download the integration file manually</button>.</span> | |
<p class="tw-pt-16-px tw-text-text-muted tw-text-12-px tw-font-400">Not the right file? <button class="tw-underline tw-underline-offset-1 tw-cursor-pointer" (click)="onRetryClick()" aria-label="Retry with different file path">Try again</button>.</p> | |
</div> | |
</div> |
configurationHeaderText: 'Connect to NetSuite subsidary', | ||
configurationSubHeaderText: 'Expenses will be posted to the NetSuite subsidary selected here. You can\'t change the subsidary once they\'re configured.', | ||
configurationHeaderText: 'Connect to QuickBooks Desktop', | ||
configurationSubHeaderText: 'Connect to QuickBooks desktop by completing the following steps.', | ||
stepName: 'Connect to QuickBooks Desktop', | ||
subLabel: 'Provide your credentials to establish a secure connection between your Expense Management and NetSuite account' |
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.
Fix inconsistent reference to NetSuite in subLabel.
The subLabel text still refers to "NetSuite account" which is inconsistent with the QuickBooks Desktop integration context.
Apply this diff to fix the inconsistency:
- subLabel: 'Provide your credentials to establish a secure connection between your Expense Management and NetSuite account'
+ subLabel: 'Provide your credentials to establish a secure connection between your Expense Management and QuickBooks Desktop account'
📝 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.
subLabel: 'Provide your credentials to establish a secure connection between your Expense Management and NetSuite account' | |
subLabel: 'Provide your credentials to establish a secure connection between your Expense Management and QuickBooks Desktop account' |
stepName: 'Connect to QuickBooks Desktop', | ||
subLabel: 'Expenses will be posted to the NetSuite Tenant Mapping selected here. Once configured, you can not change ' + brandingConfig.brandName + ' organization or Tenant Mapping.' |
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.
Fix inconsistent reference to NetSuite in subLabel text.
The subLabel text still references "NetSuite Tenant Mapping" which is inconsistent with the QuickBooks Desktop context.
Apply this diff to fix the inconsistency:
- subLabel: 'Expenses will be posted to the NetSuite Tenant Mapping selected here. Once configured, you can not change ' + brandingConfig.brandName + ' organization or Tenant Mapping.'
+ subLabel: 'Expenses will be posted to the QuickBooks Desktop configuration selected here. Once configured, you can not change ' + brandingConfig.brandName + ' organization or configuration.'
📝 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.
stepName: 'Connect to QuickBooks Desktop', | |
subLabel: 'Expenses will be posted to the NetSuite Tenant Mapping selected here. Once configured, you can not change ' + brandingConfig.brandName + ' organization or Tenant Mapping.' | |
stepName: 'Connect to QuickBooks Desktop', | |
subLabel: 'Expenses will be posted to the QuickBooks Desktop configuration selected here. Once configured, you can not change ' + brandingConfig.brandName + ' organization or configuration.' |
* feat: qbd-direct-onboarding-pre-requisite implementation * styling changes * unit test fix * step footer contentt fix * pre requisite Ui updation * PR comments fix * PR comments fix * feat: Download qwd file UI changes (#1059) * feat: Download qwd file UI changes * download file Ui updation * download file Ui updation * download file Ui updation * download file Ui updation * feat: qbd connector setup UI changes (#1060) * feat: qbd connector setup UI changes * Merge branch qbd-direct-onboarding-download-file-UI into qbd-direct-step-connector-UI * feat: Qbd direct connection data sync UI changes (#1061) * feat: Qbd direct connection data sync UI changes * input made required * svg update * feat: qbd direct pre requisite ts changes (#1062) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes (#1063) * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes (#1064) * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes (#1065) * PR comment fix * PR comment fix * Qbd direct connector data sync up ts (#1070) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes * feat: QBD direct main connection page business logic (#1066) * feat: QBD direct main connection page business logic * onboarding connection ts changes * onboarding connection ts changes --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]>
* feat: qbd direct onboarding landing page * feat: qbd-direct-onboarding-pre-requisite implementation * PR comments fix * PR fix * updateWorkspaceOnboardingState service return type update * qbd direct logo update * feat: qbd-direct onboarding prerequisite UI implementation (#1058) * feat: qbd-direct-onboarding-pre-requisite implementation * styling changes * unit test fix * step footer contentt fix * pre requisite Ui updation * PR comments fix * PR comments fix * feat: Download qwd file UI changes (#1059) * feat: Download qwd file UI changes * download file Ui updation * download file Ui updation * download file Ui updation * download file Ui updation * feat: qbd connector setup UI changes (#1060) * feat: qbd connector setup UI changes * Merge branch qbd-direct-onboarding-download-file-UI into qbd-direct-step-connector-UI * feat: Qbd direct connection data sync UI changes (#1061) * feat: Qbd direct connection data sync UI changes * input made required * svg update * feat: qbd direct pre requisite ts changes (#1062) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes (#1063) * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes (#1064) * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes (#1065) * PR comment fix * PR comment fix * Qbd direct connector data sync up ts (#1070) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes * feat: QBD direct main connection page business logic (#1066) * feat: QBD direct main connection page business logic * onboarding connection ts changes * onboarding connection ts changes --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]>
* feat: onboarding basic setup * feat: qbd direct onboarding landing page (#1056) * feat: qbd direct onboarding landing page * feat: qbd-direct-onboarding-pre-requisite implementation * PR comments fix * PR fix * updateWorkspaceOnboardingState service return type update * qbd direct logo update * feat: qbd-direct onboarding prerequisite UI implementation (#1058) * feat: qbd-direct-onboarding-pre-requisite implementation * styling changes * unit test fix * step footer contentt fix * pre requisite Ui updation * PR comments fix * PR comments fix * feat: Download qwd file UI changes (#1059) * feat: Download qwd file UI changes * download file Ui updation * download file Ui updation * download file Ui updation * download file Ui updation * feat: qbd connector setup UI changes (#1060) * feat: qbd connector setup UI changes * Merge branch qbd-direct-onboarding-download-file-UI into qbd-direct-step-connector-UI * feat: Qbd direct connection data sync UI changes (#1061) * feat: Qbd direct connection data sync UI changes * input made required * svg update * feat: qbd direct pre requisite ts changes (#1062) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes (#1063) * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes (#1064) * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes (#1065) * PR comment fix * PR comment fix * Qbd direct connector data sync up ts (#1070) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes * feat: QBD direct main connection page business logic (#1066) * feat: QBD direct main connection page business logic * onboarding connection ts changes * onboarding connection ts changes --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]>
* feat: checkbox button creation * PR comments fix * feat: onboarding basic setup (#1055) * feat: onboarding basic setup * feat: qbd direct onboarding landing page (#1056) * feat: qbd direct onboarding landing page * feat: qbd-direct-onboarding-pre-requisite implementation * PR comments fix * PR fix * updateWorkspaceOnboardingState service return type update * qbd direct logo update * feat: qbd-direct onboarding prerequisite UI implementation (#1058) * feat: qbd-direct-onboarding-pre-requisite implementation * styling changes * unit test fix * step footer contentt fix * pre requisite Ui updation * PR comments fix * PR comments fix * feat: Download qwd file UI changes (#1059) * feat: Download qwd file UI changes * download file Ui updation * download file Ui updation * download file Ui updation * download file Ui updation * feat: qbd connector setup UI changes (#1060) * feat: qbd connector setup UI changes * Merge branch qbd-direct-onboarding-download-file-UI into qbd-direct-step-connector-UI * feat: Qbd direct connection data sync UI changes (#1061) * feat: Qbd direct connection data sync UI changes * input made required * svg update * feat: qbd direct pre requisite ts changes (#1062) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes (#1063) * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes (#1064) * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes (#1065) * PR comment fix * PR comment fix * Qbd direct connector data sync up ts (#1070) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes * feat: QBD direct main connection page business logic (#1066) * feat: QBD direct main connection page business logic * onboarding connection ts changes * onboarding connection ts changes --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]>
Description
feat: Download qwd file UI changes
Clickup
https://app.clickup.com/t/86cwzcemu
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Style