feat: new integrations landing page (/landing_v2)
#1151
Conversation
* feat: create new landing page and add header and tab switcher * feat: new app tile structure + remove extra content (#1140) * feat: new app tile structure + remove extra content * feat: responsive grid layout + new tile layout (#1141) * feat: responsive grid layout + new tile layout * feat: add CTA and shadow animation on tile hover (#1142) * fix: update images * feat: add CTA and shadow animation on tile hover * feat: UI for the new "Conencted" badge (#1144) * feat: UI for the new "Conencted" badge * feat: fetch data from integrations API and show connected apps (#1143) * feat: fetch data from integrations API and show connected apps * fix: update route name and keep `exposeC1Apps` (#1145) * fix: update route name and keep `exposeC1Apps` * fix: use new logos in landing-v2 only Use original images everywhere else. * fix: incorrect URL for `/integrations` call and rendering errors (#1149)
WalkthroughThis pull request introduces several enhancements to the application's integration and theming capabilities. The changes include adding a new type alias for integration app keys, expanding the theme options, creating a new Changes
Sequence DiagramsequenceDiagram
participant User
participant LandingV2Component
participant IntegrationsService
participant ApiService
User->>LandingV2Component: Open Integrations Page
LandingV2Component->>IntegrationsService: getIntegrations()
IntegrationsService->>ApiService: GET /integrations/
ApiService-->>IntegrationsService: Return Integration[]
IntegrationsService-->>LandingV2Component: Provide Integrations
LandingV2Component->>User: Display Integrations Grid
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
🪧 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 (
|
JustARatherRidiculouslyLongUsername
changed the title
/landing_v2)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (7)
src/app/integrations/landing-v2/landing-v2.component.ts (2)
196-206: SimplifyloginAndRedirectToInAppIntegrationusing a mapping for login methods.The series of
if/elsestatements can be refactored into a mapping for better readability and scalability.Here's how you can refactor the method:
private loginAndRedirectToInAppIntegration(redirectUri: string, inAppIntegration: InAppIntegration): void { const authCode = redirectUri.split('code=')[1].split('&')[0]; const loginMethods: { [key in InAppIntegration]?: () => Observable<Token> } = { [InAppIntegration.INTACCT]: () => this.siAuthService.loginWithAuthCode(authCode), [InAppIntegration.QBO]: () => this.qboAuthService.loginWithAuthCode(authCode), [InAppIntegration.XERO]: () => this.xeroAuthService.login(authCode), [InAppIntegration.NETSUITE]: () => this.nsAuthService.loginWithAuthCode(authCode) }; const login$ = loginMethods[inAppIntegration]?.(); if (!login$) { return; } login$.subscribe((token: Token) => { const user: MinimalUser = { 'email': token.user.email, 'access_token': token.access_token, 'refresh_token': token.refresh_token, 'full_name': token.user.full_name, 'user_id': token.user.user_id, 'org_id': token.user.org_id, 'org_name': token.user.org_name }; this.storageService.set('user', user); this.openInAppIntegration(inAppIntegration); }); }
165-165: Fix typo in comment.There's a typo in the comment; "shouln't" should be "shouldn't".
Apply this diff to correct the typo:
- // TS catch-all (shouln't reach here) + // TS catch-all (shouldn't reach here)src/app/core/models/integrations/integrations.model.ts (1)
3-12: Consider adding field constraints and documentation.The Integration type looks good but could benefit from:
- JSDoc comments describing the purpose of each field
- String literal types for the
typefield to restrict possible valuesExample enhancement:
+ /** Represents an integration between Fyle and a third-party application */ export type Integration = { + /** Unique identifier for the integration */ id: number; org_id: string; org_name: string; tpa_id: string; tpa_name: string; - type: string; + type: 'ACCOUNTING' | 'HRMS' | 'TRAVEL'; is_active: boolean; is_beta: boolean; }src/app/integrations/integrations-routing.module.ts (1)
16-19: Consider route organization and default path.While the new route is correctly added, consider:
- Adding a redirect from the empty path to either 'landing' or 'landing_v2'
- Documenting the plan for maintaining both landing routes
Example enhancement:
children: [ + { + path: '', + redirectTo: 'landing_v2', + pathMatch: 'full' + }, { path: 'landing', component: LandingComponent }, { path: 'landing_v2', component: LandingV2Component },src/app/integrations/landing-v2/landing-v2.component.scss (1)
68-68: Use Tailwind's shadow utilities for consistency.Replace hardcoded box-shadow value with Tailwind's shadow utilities for better maintainability.
- box-shadow: 0px 4px 4px 0px rgba(44, 48, 78, 0.10); + @apply tw-shadow-md;src/app/integrations/landing-v2/landing-v2.component.html (2)
11-12: Consider extracting support email to configuration.The support email address is hardcoded. Consider moving it to a configuration file for easier maintenance.
39-237: Optimize list rendering with trackBy function.Add trackBy function to optimize the rendering of integration cards.
In the component:
trackByApp(index: number, app: string): string { return app; }In the template:
- <div *ngIf="isAppShown('NETSUITE')"> + <div *ngIf="isAppShown('NETSUITE')" [trackBy]="trackByApp">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
src/assets/logos/bamboo-hr-logo.pngis excluded by!**/*.pngsrc/assets/logos/intacct-logo-new.pngis excluded by!**/*.pngsrc/assets/logos/netsuite-logo-new.pngis excluded by!**/*.pngsrc/assets/logos/sage300-logo.pngis excluded by!**/*.pngsrc/assets/logos/travelperk-logo.pngis excluded by!**/*.pngsrc/assets/logos/xero-logo-new.pngis excluded by!**/*.png
📒 Files selected for processing (11)
src/app/core/models/enum/enum.model.ts(2 hunks)src/app/core/models/integrations/integrations.model.ts(1 hunks)src/app/core/services/common/integrations.service.spec.ts(1 hunks)src/app/core/services/common/integrations.service.ts(1 hunks)src/app/integrations/integrations-routing.module.ts(2 hunks)src/app/integrations/integrations.module.ts(2 hunks)src/app/integrations/landing-v2/landing-v2.component.html(1 hunks)src/app/integrations/landing-v2/landing-v2.component.scss(1 hunks)src/app/integrations/landing-v2/landing-v2.component.spec.ts(1 hunks)src/app/integrations/landing-v2/landing-v2.component.ts(1 hunks)src/app/shared/components/core/badge/badge.component.scss(1 hunks)
🔇 Additional comments (5)
src/app/integrations/integrations.module.ts (1)
22-23: LGTM!The addition of LandingV2Component to the module declarations is clean and follows Angular best practices.
src/app/integrations/integrations-routing.module.ts (1)
16-19: Verify the migration plan for the landing pages.Since both landing routes coexist, please clarify:
- Is this intentional for A/B testing?
- What's the timeline for deprecating the old landing page?
- Are there any feature differences between the two versions?
Let's check for any documentation about the migration plan:
src/app/core/models/enum/enum.model.ts (2)
45-45: LGTM! Type alias enhances type safety.The
IntegrationAppKeytype alias provides type safety when referencing integration app keys throughout the codebase.
905-906: LGTM! Theme option addition aligns with badge usage.The addition of
SUCCESStheme option aligns with its usage in the badge component for connected apps status.src/app/shared/components/core/badge/badge.component.scss (1)
25-28: LGTM! Success theme styling follows consistent pattern.The
.theme-successclass follows the established pattern of other theme classes and provides appropriate styling for success states.
| private setupLoginWatcher(): void { | ||
| this.eventsService.sageIntacctLogin.subscribe((redirectUri: string) => { | ||
| this.loginAndRedirectToInAppIntegration(redirectUri, InAppIntegration.INTACCT); | ||
| }); | ||
|
|
||
| this.eventsService.qboLogin.subscribe((redirectUri: string) => { | ||
| this.loginAndRedirectToInAppIntegration(redirectUri, InAppIntegration.QBO); | ||
| }); | ||
|
|
||
| this.eventsService.xeroLogin.subscribe((redirectUri: string) => { | ||
| this.loginAndRedirectToInAppIntegration(redirectUri, InAppIntegration.XERO); | ||
| }); | ||
|
|
||
| this.eventsService.netsuiteLogin.subscribe((redirectUri: string) => { | ||
| this.loginAndRedirectToInAppIntegration(redirectUri, InAppIntegration.NETSUITE); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Unsubscribe from Observables in setupLoginWatcher to prevent memory leaks.
The subscriptions in setupLoginWatcher are not being unsubscribed when the component is destroyed, which can lead to memory leaks. Implement OnDestroy and manage the subscriptions using takeUntil or store the subscriptions and unsubscribe in ngOnDestroy().
Here's how you can modify the code:
import { OnDestroy } from '@angular/core';
import { Subject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';
export class LandingV2Component implements OnInit, OnDestroy {
private destroy$ = new Subject<void>();
// ...
private setupLoginWatcher(): void {
this.eventsService.sageIntacctLogin.pipe(takeUntil(this.destroy$)).subscribe((redirectUri: string) => {
this.loginAndRedirectToInAppIntegration(redirectUri, InAppIntegration.INTACCT);
});
this.eventsService.qboLogin.pipe(takeUntil(this.destroy$)).subscribe((redirectUri: string) => {
this.loginAndRedirectToInAppIntegration(redirectUri, InAppIntegration.QBO);
});
this.eventsService.xeroLogin.pipe(takeUntil(this.destroy$)).subscribe((redirectUri: string) => {
this.loginAndRedirectToInAppIntegration(redirectUri, InAppIntegration.XERO);
});
this.eventsService.netsuiteLogin.pipe(takeUntil(this.destroy$)).subscribe((redirectUri: string) => {
this.loginAndRedirectToInAppIntegration(redirectUri, InAppIntegration.NETSUITE);
});
}
ngOnDestroy(): void {
this.destroy$.next();
this.destroy$.complete();
}
}| @Component({ | ||
| selector: 'app-landing-v2', | ||
| templateUrl: './landing-v2.component.html', | ||
| styleUrl: './landing-v2.component.scss' |
There was a problem hiding this comment.
Change 'styleUrl' to 'styleUrls' in the @component decorator.
In the @Component decorator, use styleUrls (plural) instead of styleUrl (singular). This is the correct property for specifying the component's stylesheets in Angular, even if there is only one stylesheet.
Apply this diff to fix the issue:
- styleUrl: './landing-v2.component.scss'
+ styleUrls: ['./landing-v2.component.scss']📝 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.
| styleUrl: './landing-v2.component.scss' | |
| styleUrls: ['./landing-v2.component.scss'] |
| const initialState = Object.create(this.integrationTabsInitialState); | ||
|
|
||
| // Resetting to initial state and setting clicked view to true | ||
| this.integrationTabs = initialState; |
There was a problem hiding this comment.
Use object spread or Object.assign to correctly copy integrationTabsInitialState.
Using Object.create creates a new object with the passed object as its prototype, which may not copy the properties as intended. Instead, use the object spread operator {...} or Object.assign to create a shallow copy of the object.
Apply this diff to fix the issue:
- const initialState = Object.create(this.integrationTabsInitialState);
+ const initialState = { ...this.integrationTabsInitialState };📝 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.
| const initialState = Object.create(this.integrationTabsInitialState); | |
| // Resetting to initial state and setting clicked view to true | |
| this.integrationTabs = initialState; | |
| const initialState = { ...this.integrationTabsInitialState }; | |
| // Resetting to initial state and setting clicked view to true | |
| this.integrationTabs = initialState; |
| xdescribe('IntegrationsService', () => { | ||
| let service: IntegrationsService; | ||
|
|
||
| beforeEach(() => { | ||
| TestBed.configureTestingModule({}); | ||
| service = TestBed.inject(IntegrationsService); | ||
| }); | ||
|
|
||
| it('should be created', () => { | ||
| expect(service).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Enable the test suite by replacing xdescribe with describe.
The test suite is currently disabled due to the xdescribe. To ensure the tests run, remove the x prefix.
Apply this diff to enable the tests:
- xdescribe('IntegrationsService', () => {
+ describe('IntegrationsService', () => {📝 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.
| xdescribe('IntegrationsService', () => { | |
| let service: IntegrationsService; | |
| beforeEach(() => { | |
| TestBed.configureTestingModule({}); | |
| service = TestBed.inject(IntegrationsService); | |
| }); | |
| it('should be created', () => { | |
| expect(service).toBeTruthy(); | |
| }); | |
| }); | |
| describe('IntegrationsService', () => { | |
| let service: IntegrationsService; | |
| beforeEach(() => { | |
| TestBed.configureTestingModule({}); | |
| service = TestBed.inject(IntegrationsService); | |
| }); | |
| it('should be created', () => { | |
| expect(service).toBeTruthy(); | |
| }); | |
| }); |
| beforeEach(async () => { | ||
| await TestBed.configureTestingModule({ | ||
| declarations: [LandingV2Component] | ||
| }) | ||
| .compileComponents(); | ||
|
|
||
| fixture = TestBed.createComponent(LandingV2Component); | ||
| component = fixture.componentInstance; | ||
| fixture.detectChanges(); | ||
| }); |
There was a problem hiding this comment.
Add necessary imports and providers for the component's dependencies.
The test setup is missing imports and providers for services and modules that LandingV2Component depends on. Without these, the test may fail or not fully validate the component's functionality.
Consider updating the testing module configuration to include necessary imports, declarations, and providers. For example:
beforeEach(async () => {
await TestBed.configureTestingModule({
declarations: [LandingV2Component],
imports: [/* Add necessary modules here */],
providers: [/* Add necessary services here */]
})
.compileComponents();
fixture = TestBed.createComponent(LandingV2Component);
component = fixture.componentInstance;
fixture.detectChanges();
});Ensure all dependencies are properly mocked or provided to facilitate effective unit testing.
| xdescribe('LandingV2Component', () => { | ||
| let component: LandingV2Component; | ||
| let fixture: ComponentFixture<LandingV2Component>; | ||
|
|
||
| beforeEach(async () => { | ||
| await TestBed.configureTestingModule({ | ||
| declarations: [LandingV2Component] | ||
| }) | ||
| .compileComponents(); | ||
|
|
||
| fixture = TestBed.createComponent(LandingV2Component); | ||
| component = fixture.componentInstance; | ||
| fixture.detectChanges(); | ||
| }); | ||
|
|
||
| it('should create', () => { | ||
| expect(component).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Enable the test suite by replacing xdescribe with describe.
The test suite is currently disabled due to the xdescribe. To execute the tests and validate the LandingV2Component, remove the x prefix.
Apply this diff to enable the tests:
- xdescribe('LandingV2Component', () => {
+ describe('LandingV2Component', () => {📝 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.
| xdescribe('LandingV2Component', () => { | |
| let component: LandingV2Component; | |
| let fixture: ComponentFixture<LandingV2Component>; | |
| beforeEach(async () => { | |
| await TestBed.configureTestingModule({ | |
| declarations: [LandingV2Component] | |
| }) | |
| .compileComponents(); | |
| fixture = TestBed.createComponent(LandingV2Component); | |
| component = fixture.componentInstance; | |
| fixture.detectChanges(); | |
| }); | |
| it('should create', () => { | |
| expect(component).toBeTruthy(); | |
| }); | |
| }); | |
| describe('LandingV2Component', () => { | |
| let component: LandingV2Component; | |
| let fixture: ComponentFixture<LandingV2Component>; | |
| beforeEach(async () => { | |
| await TestBed.configureTestingModule({ | |
| declarations: [LandingV2Component] | |
| }) | |
| .compileComponents(); | |
| fixture = TestBed.createComponent(LandingV2Component); | |
| component = fixture.componentInstance; | |
| fixture.detectChanges(); | |
| }); | |
| it('should create', () => { | |
| expect(component).toBeTruthy(); | |
| }); | |
| }); |
| getIntegrations(): Observable<Integration[]> { | ||
| this.helper.setBaseApiURL(AppUrl.INTEGRATION); | ||
| return this.apiService.get(`/integrations/`, {}); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and consider caching.
The getIntegrations method should handle API errors and implement caching for better performance.
Consider implementing:
+ private integrationsCache$ = new ReplaySubject<Integration[]>(1);
getIntegrations(): Observable<Integration[]> {
this.helper.setBaseApiURL(AppUrl.INTEGRATION);
- return this.apiService.get(`/integrations/`, {});
+ return this.apiService.get<Integration[]>(`/integrations/`, {}).pipe(
+ catchError(error => {
+ console.error('Failed to fetch integrations:', error);
+ return throwError(() => new Error('Failed to fetch integrations. Please try again.'));
+ }),
+ tap(integrations => this.integrationsCache$.next(integrations)),
+ shareReplay(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.
| getIntegrations(): Observable<Integration[]> { | |
| this.helper.setBaseApiURL(AppUrl.INTEGRATION); | |
| return this.apiService.get(`/integrations/`, {}); | |
| } | |
| private integrationsCache$ = new ReplaySubject<Integration[]>(1); | |
| getIntegrations(): Observable<Integration[]> { | |
| this.helper.setBaseApiURL(AppUrl.INTEGRATION); | |
| return this.apiService.get<Integration[]>(`/integrations/`, {}).pipe( | |
| catchError(error => { | |
| console.error('Failed to fetch integrations:', error); | |
| return throwError(() => new Error('Failed to fetch integrations. Please try again.')); | |
| }), | |
| tap(integrations => this.integrationsCache$.next(integrations)), | |
| shareReplay(1) | |
| ); | |
| } |
| .landing-v2--accounting-app:hover { | ||
| box-shadow: 0px 4px 4px 0px rgba(44, 48, 78, 0.10); | ||
| .btn-connect { | ||
| @apply tw-block; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider accessibility for hover-only button visibility.
The .btn-connect button is only visible on hover, which may cause accessibility issues for keyboard and screen reader users.
Consider making the button always visible or implementing keyboard focus styles:
- .btn-connect {
- @apply tw-block;
- }
+ .btn-connect {
+ @apply tw-block;
+ }
+}
+
+.landing-v2--accounting-app:focus-within {
+ box-shadow: 0px 4px 4px 0px rgba(44, 48, 78, 0.10);
+ .btn-connect {
+ @apply tw-block;
+ }📝 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.
| .landing-v2--accounting-app:hover { | |
| box-shadow: 0px 4px 4px 0px rgba(44, 48, 78, 0.10); | |
| .btn-connect { | |
| @apply tw-block; | |
| } | |
| .landing-v2--accounting-app:hover { | |
| box-shadow: 0px 4px 4px 0px rgba(44, 48, 78, 0.10); | |
| .btn-connect { | |
| @apply tw-block; | |
| } | |
| } | |
| .landing-v2--accounting-app:focus-within { | |
| box-shadow: 0px 4px 4px 0px rgba(44, 48, 78, 0.10); | |
| .btn-connect { | |
| @apply tw-block; | |
| } | |
| } |
| Accounting | ||
| </span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <div *ngIf="isAppShown('INTACCT')" class="landing-v2--accounting-app" | ||
| (click)="openAccountingIntegrationApp(AccountingIntegrationApp.SAGE_INTACCT)"> | ||
| <div class="tw-flex tw-justify-between tw-items-center"> | ||
| <img src="assets/logos/intacct-logo-new.png" /> | ||
| @if (isAppConnected('INTACCT')) { | ||
| <app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge> | ||
| } @else { | ||
| <button class="btn-connect">Connect</button> | ||
| } | ||
| </div> | ||
| <div> | ||
| <span class="landing-v2--accounting-app-name"> | ||
| Sage Intacct | ||
| </span> | ||
| <span class="landing-v2--accounting-app-type"> | ||
| Accounting | ||
| </span> | ||
| </div> | ||
| </div> | ||
| <div *ngIf="isAppShown('QBO')" class="landing-v2--accounting-app" | ||
| (click)="openAccountingIntegrationApp(AccountingIntegrationApp.QBO)"> | ||
| <div class="tw-flex tw-justify-between tw-items-center"> | ||
| <img src="assets/logos/quickbooks-logo.png" class="!tw-h-[30.7px]" /> | ||
| @if (isAppConnected('QBO')) { | ||
| <app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge> | ||
| } @else { | ||
| <button class="btn-connect">Connect</button> | ||
| } | ||
| </div> | ||
| <div> | ||
| <span class="landing-v2--accounting-app-name"> | ||
| QuickBooks Online | ||
| </span> | ||
| <span class="landing-v2--accounting-app-type"> | ||
| Accounting | ||
| </span> | ||
| </div> | ||
| </div> | ||
| <div *ngIf="isAppShown('XERO')" class="landing-v2--accounting-app" | ||
| (click)="openAccountingIntegrationApp(AccountingIntegrationApp.XERO)"> | ||
| <div class="tw-flex tw-justify-between tw-items-center"> | ||
| <img src="assets/logos/xero-logo-new.png" /> | ||
| @if (isAppConnected('XERO')) { | ||
| <app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge> | ||
| } @else { | ||
| <button class="btn-connect">Connect</button> | ||
| } | ||
| </div> | ||
| <div> | ||
| <span class="landing-v2--accounting-app-name"> | ||
| Xero | ||
| </span> | ||
| <span class="landing-v2--accounting-app-type"> | ||
| Accounting | ||
| </span> | ||
| </div> | ||
| </div> | ||
| <div *ngIf="isAppShown('QBD')" class="landing-v2--accounting-app" | ||
| (click)="openInAppIntegration(InAppIntegration.QBD)"> | ||
| <div class="tw-flex tw-justify-between tw-items-center"> | ||
| <img src="assets/logos/quickbooks-logo.png" class="!tw-h-[30.7px]" /> | ||
| @if (isAppConnected('QBD')) { | ||
| <app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge> | ||
| } @else { | ||
| <button class="btn-connect">Connect</button> | ||
| } | ||
| </div> | ||
| <div> | ||
| <span class="landing-v2--accounting-app-name"> | ||
| QuickBooks Desktop (IIF) | ||
| </span> | ||
| <span class="landing-v2--accounting-app-type"> | ||
| Accounting | ||
| </span> | ||
| </div> | ||
| </div> | ||
| <!-- Direct --> | ||
| <div *ngIf="isAppShown('QBD_DIRECT')" class="landing-v2--accounting-app" | ||
| (click)="openInAppIntegration(InAppIntegration.QBD_DIRECT)"> | ||
| <div class="tw-flex tw-justify-between tw-items-center"> | ||
| <img src="assets/logos/quickbooks-logo.png" class="!tw-h-[30.7px]" /> | ||
| @if (isAppConnected('QBD_DIRECT')) { | ||
| <app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge> | ||
| } @else { | ||
| <button class="btn-connect">Connect</button> | ||
| } | ||
| </div> | ||
| <span class="landing-v2--accounting-app-name tw-items-center tw-gap-4"> | ||
| <div> | ||
| QuickBooks Desktop (Web Connector) | ||
| <div class="landing-v2--accounting-app-type">Accounting</div> | ||
| </div> | ||
| <app-badge [theme]="ThemeOption.DARK" text="Beta"></app-badge> | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract integration card to a separate component.
The integration card pattern is repeated multiple times. Consider extracting it to a reusable component to reduce code duplication and improve maintainability.
Create a new component IntegrationCardComponent:
@Component({
selector: 'app-integration-card',
template: `
<div class="landing-v2--accounting-app" (click)="onCardClick()">
<div class="tw-flex tw-justify-between tw-items-center">
<img [src]="logoSrc" [class]="logoClass" />
<app-badge *ngIf="isConnected" text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
<button *ngIf="!isConnected" class="btn-connect">Connect</button>
</div>
<div>
<span class="landing-v2--accounting-app-name">
{{ name }}
</span>
<span class="landing-v2--accounting-app-type">
{{ type }}
</span>
</div>
</div>
`
})
export class IntegrationCardComponent {
@Input() name: string;
@Input() type: string;
@Input() logoSrc: string;
@Input() logoClass: string;
@Input() isConnected: boolean;
@Output() connect = new EventEmitter<void>();
}| <div *ngIf="!exposeC1Apps" class="tw-flex tw-text-14-px tw-border-y tw-border-separator"> | ||
| <div class="landing-v2--tab" [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.ALL}" | ||
| (click)="switchView(IntegrationsView.ALL)"> | ||
| All | ||
| <p *ngIf="integrationTabs.ALL" class="landing-v2--active-tag"></p> | ||
| </div> | ||
| <div class="landing-v2--tab" [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.ACCOUNTING}" | ||
| (click)="switchView(IntegrationsView.ACCOUNTING)"> | ||
| Accounting | ||
| <p *ngIf="integrationTabs.ACCOUNTING" class="landing-v2--active-tag"></p> | ||
| </div> | ||
| <div class="landing-v2--tab" [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.HRMS}" | ||
| (click)="switchView(IntegrationsView.HRMS)"> | ||
| HRMS | ||
| <p *ngIf="integrationTabs.HRMS" class="landing-v2--active-tag"></p> | ||
| </div> | ||
| <div *ngIf="org.allow_travelperk" class="landing-v2--tab" | ||
| [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.TRAVEL}" | ||
| (click)="switchView(IntegrationsView.TRAVEL)"> | ||
| Travel | ||
| <p *ngIf="integrationTabs.TRAVEL" class="landing-v2--active-tag"></p> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add ARIA labels for better accessibility.
The tab navigation needs proper ARIA attributes for better screen reader support.
- <div *ngIf="!exposeC1Apps" class="tw-flex tw-text-14-px tw-border-y tw-border-separator">
+ <div *ngIf="!exposeC1Apps" class="tw-flex tw-text-14-px tw-border-y tw-border-separator" role="tablist" aria-label="Integration categories">
- <div class="landing-v2--tab"
+ <div class="landing-v2--tab" role="tab" [attr.aria-selected]="integrationTabs.ALL" [attr.tabindex]="integrationTabs.ALL ? 0 : -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.
| <div *ngIf="!exposeC1Apps" class="tw-flex tw-text-14-px tw-border-y tw-border-separator"> | |
| <div class="landing-v2--tab" [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.ALL}" | |
| (click)="switchView(IntegrationsView.ALL)"> | |
| All | |
| <p *ngIf="integrationTabs.ALL" class="landing-v2--active-tag"></p> | |
| </div> | |
| <div class="landing-v2--tab" [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.ACCOUNTING}" | |
| (click)="switchView(IntegrationsView.ACCOUNTING)"> | |
| Accounting | |
| <p *ngIf="integrationTabs.ACCOUNTING" class="landing-v2--active-tag"></p> | |
| </div> | |
| <div class="landing-v2--tab" [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.HRMS}" | |
| (click)="switchView(IntegrationsView.HRMS)"> | |
| HRMS | |
| <p *ngIf="integrationTabs.HRMS" class="landing-v2--active-tag"></p> | |
| </div> | |
| <div *ngIf="org.allow_travelperk" class="landing-v2--tab" | |
| [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.TRAVEL}" | |
| (click)="switchView(IntegrationsView.TRAVEL)"> | |
| Travel | |
| <p *ngIf="integrationTabs.TRAVEL" class="landing-v2--active-tag"></p> | |
| </div> | |
| </div> | |
| <div *ngIf="!exposeC1Apps" class="tw-flex tw-text-14-px tw-border-y tw-border-separator" role="tablist" aria-label="Integration categories"> | |
| <div class="landing-v2--tab" role="tab" [attr.aria-selected]="integrationTabs.ALL" [attr.tabindex]="integrationTabs.ALL ? 0 : -1" | |
| [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.ALL}" | |
| (click)="switchView(IntegrationsView.ALL)"> | |
| All | |
| <p *ngIf="integrationTabs.ALL" class="landing-v2--active-tag"></p> | |
| </div> | |
| <div class="landing-v2--tab" [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.ACCOUNTING}" | |
| (click)="switchView(IntegrationsView.ACCOUNTING)"> | |
| Accounting | |
| <p *ngIf="integrationTabs.ACCOUNTING" class="landing-v2--active-tag"></p> | |
| </div> | |
| <div class="landing-v2--tab" [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.HRMS}" | |
| (click)="switchView(IntegrationsView.HRMS)"> | |
| HRMS | |
| <p *ngIf="integrationTabs.HRMS" class="landing-v2--active-tag"></p> | |
| </div> | |
| <div *ngIf="org.allow_travelperk" class="landing-v2--tab" | |
| [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.TRAVEL}" | |
| (click)="switchView(IntegrationsView.TRAVEL)"> | |
| Travel | |
| <p *ngIf="integrationTabs.TRAVEL" class="landing-v2--active-tag"></p> | |
| </div> | |
| </div> |
|
feat: create new landing page and add header and tab switcher
feat: new app tile structure + remove extra content (feat: new app tile structure + remove extra content #1140)
feat: new app tile structure + remove extra content
feat: responsive grid layout + new tile layout (feat: responsive grid layout + new tile layout #1141)
feat: responsive grid layout + new tile layout
feat: add CTA and shadow animation on tile hover (feat: add CTA and shadow animation on tile hover #1142)
fix: update images
feat: add CTA and shadow animation on tile hover
feat: UI for the new "Conencted" badge (feat: UI for the new "Conencted" badge #1144)
feat: UI for the new "Conencted" badge
feat: fetch data from integrations API and show connected apps (feat: fetch data from integrations API and show connected apps #1143)
feat: fetch data from integrations API and show connected apps
fix: update route name and keep
exposeC1Apps(fix: update route name and keepexposeC1Apps#1145)fix: update route name and keep
exposeC1Appsfix: use new logos in landing-v2 only
Use original images everywhere else.
/integrationscall and rendering errors (fix: incorrect URL for/integrationscall and rendering errors #1149)Clickup
https://app.clickup.com/t/86cxhef57
Summary by CodeRabbit
New Features
Enhancements
New Components
Technical Improvements