[Feat] Create Reusable Component and Add Workspace Links #8383
Conversation
WalkthroughThe changes in this pull request involve modifications to the authentication module and login components of the desktop UI library. Key updates include the export of the Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (13)
packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.scss (2)
3-5: Consider using a variable for the margin valueThe top margin of 21px seems specific. Consider using a SCSS variable for this value to improve maintainability and consistency across the application.
+$social-links-top-margin: 21px; .links { - margin-top: 21px; + margin-top: $social-links-top-margin; @include social-links-style;This change would make it easier to adjust the spacing consistently if needed in the future.
6-9: Good use of nesting, consider using variables for marginsThe nesting of
.socialswithin.linksis a good use of SCSS features. However, consider using variables for the margin values to improve maintainability:+$socials-top-margin: 15px; +$socials-bottom-margin: 25px; .links { // ... & .socials { - margin-top: 15px; - margin-bottom: 25px; + margin-top: $socials-top-margin; + margin-bottom: $socials-bottom-margin; } }This change would make it easier to adjust the spacing consistently if needed in the future.
packages/desktop-ui-lib/src/lib/login/shared/ui/logo/logo.component.ts (2)
10-15: LGTM: Class definition and constructor are well-structured.The component class is correctly defined, and dependency injection is properly used. However, consider improving type safety:
Consider creating a specific interface for the environment configuration instead of using
any:interface GauzyEnvironment { PLATFORM_LOGO: string; // Add other environment properties as needed } constructor( private readonly _domSanitizer: DomSanitizer, @Inject(GAUZY_ENV) private readonly _environment: GauzyEnvironment ) {}This will provide better type checking and autocompletion for environment properties.
17-19: LGTM: Getter method correctly handles URL sanitization.The
logoUrlgetter appropriately usesDomSanitizerto handle the logo URL safely. However, consider adding error handling:Add a check for the existence of
PLATFORM_LOGOin the environment object:public get logoUrl(): SafeResourceUrl { if (!this._environment.PLATFORM_LOGO) { console.warn('PLATFORM_LOGO is not defined in the environment'); return this._domSanitizer.bypassSecurityTrustResourceUrl(''); } return this._domSanitizer.bypassSecurityTrustResourceUrl(this._environment.PLATFORM_LOGO); }This will prevent potential runtime errors if the logo URL is not defined in the environment.
packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.html (1)
3-5: LGTM: Good accessibility and internationalization practices.The use of
aria-labelfor the social links container and the translation pipe for the text improves accessibility and internationalization. The structure is appropriate for grouping social links.Consider adding a heading element (e.g.,
<h2>) for the "Social SignIn" text to improve the semantic structure of the document. This can further enhance accessibility and SEO. For example:<div class="links"> <h2 aria-label="Social SignIn">{{ 'LOGIN_PAGE.OR_SIGN_IN_WITH' | translate }}</h2> <div class="socials"> <!-- ... --> </div> </div>packages/desktop-ui-lib/src/lib/login/login.module.ts (1)
Line range hint
1-41: Consider organizing imports for better readabilityWhile the current import structure is functional, consider grouping related imports together for improved readability and maintainability. Here's a suggested organization:
- Angular core imports
- Angular module imports
- Nebular imports
- Local module imports
- Component imports
This organization can make it easier to manage imports as the module grows.
Here's an example of how you could reorganize the imports:
// Angular core imports import { NgModule } from '@angular/core'; import { CommonModule } from '@angular/common'; import { FormsModule } from '@angular/forms'; import { RouterModule } from '@angular/router'; // Nebular imports import { NbAuthModule } from '@nebular/auth'; import { NbAlertModule, NbButtonModule, NbCardModule, NbCheckboxModule, NbFormFieldModule, NbIconModule, NbInputModule } from '@nebular/theme'; // Local module imports import { DesktopDirectiveModule } from '../directives/desktop-directive.module'; import { LanguageModule } from '../language/language.module'; import { SwitchThemeModule } from '../theme-selector/switch-theme/switch-theme.module'; // Component imports import { NgxLoginComponent } from './login.component'; import { LogoComponent } from './shared/ui/logo/logo.component'; import { SocialLinksComponent } from './shared/ui/social-links/social-links.component';packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.ts (2)
1-4: LGTM with a suggestion for improvementThe imports look good and cover all necessary dependencies for the component's functionality. The use of
UntilDestroyis a good practice for managing subscriptions.However, consider moving the
socialLinksconstant to a separate shared module to avoid potential circular dependencies between the auth and login modules.
28-39: LGTM with a minor documentation improvementThe
SocialLinksComponentclass is well-structured and follows Angular best practices. The use of an Observable forsocialLinks$allows for reactive programming, which is a good approach.However, the JSDoc comment for the
ngOnInitmethod is incomplete. Consider adding a brief description of what the method does.Suggestion for improving the JSDoc comment:
/** * Initializes the component by setting up the socialLinks$ Observable. * This method is called once the component is initialized. */ ngOnInit(): void { this.socialLinks$ = of(socialLinks); }packages/desktop-ui-lib/src/lib/login/login.component.ts (1)
Line range hint
1-41: Summary of changes and potential impactThe modifications to
NgxLoginComponentprimarily involve:
- Simplification of the constructor by removing
DomSanitizerandGAUZY_ENVdependencies.- Removal of the
logoUrlgetter method.These changes suggest a shift in how environment-specific configurations and URL sanitization are handled. While the core login functionality appears to be preserved, it's important to:
- Verify that the removed functionality is appropriately managed elsewhere in the application if still needed.
- Ensure that any components or templates that previously relied on the
logoUrlgetter have been updated accordingly.- Review the application's error handling and security measures, particularly regarding URL sanitization, given the removal of
DomSanitizer.Consider documenting these architectural changes, especially regarding how environment-specific configurations and URL sanitization are now handled, to maintain clear communication within the development team and ease future maintenance.
packages/desktop-ui-lib/src/lib/auth/auth.module.ts (1)
15-15: Approved: ExportingsocialLinksenhances reusability, but consider long-term implications.The change to export the
socialLinksconstant is straightforward and can improve code reuse. This modification allows other parts of the application to access and utilize the same social authentication configuration, promoting consistency.However, keep in mind:
- This increases the public API surface of the auth module.
- It may lead to tighter coupling if many components start depending on this exported constant.
In the future, consider:
- Documenting the intended usage of this exported constant.
- Monitoring its usage to prevent over-reliance across the application.
- Evaluating if a more robust solution (e.g., a service) might be needed if the configuration becomes more complex or dynamic.
packages/desktop-ui-lib/src/lib/login/login.component.scss (2)
329-330: Good use of relative units, consider small horizontal paddingThe change to use
remunits for vertical padding is good for responsiveness. However, completely removing the horizontal padding might cause content to touch the screen edges on very small devices.Consider adding a small horizontal padding, e.g.:
- padding: 1rem 0; + padding: 1rem 0.5rem;This will maintain some breathing space on the sides while still allowing for a more compact mobile layout.
Line range hint
391-394: Good centering technique, consider flexible vertical positioningThe horizontal centering using
left: 50%andtransform: translate(-50%)is a solid approach. Thez-index: 6ensures the element remains visible above other content.Consider using a more flexible approach for vertical positioning:
.demo-credentials-select { - top: 83px; + top: 10vh; /* Or use a percentage of the parent's height */ left: 50%; transform: translate(-50%); z-index: 6; }This change will make the vertical positioning more adaptable to different screen sizes while maintaining the intended layout.
packages/desktop-ui-lib/src/lib/login/login.component.html (1)
130-132: Consider addingaria-labelto the login magic link for accessibilityAdding an
aria-labelto the<a>tag improves accessibility for screen readers.Apply this diff to include the
aria-label:<a routerLink="/auth/login-magic" + aria-label="{{ 'LOGIN_PAGE.LOGIN_MAGIC.TITLE' | translate }}"> {{ 'LOGIN_PAGE.LOGIN_MAGIC.TITLE' | translate }} </a>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
- packages/desktop-ui-lib/src/lib/auth/auth.module.ts (1 hunks)
- packages/desktop-ui-lib/src/lib/constants/app.constants.ts (1 hunks)
- packages/desktop-ui-lib/src/lib/login/login.component.html (2 hunks)
- packages/desktop-ui-lib/src/lib/login/login.component.scss (2 hunks)
- packages/desktop-ui-lib/src/lib/login/login.component.ts (1 hunks)
- packages/desktop-ui-lib/src/lib/login/login.module.ts (2 hunks)
- packages/desktop-ui-lib/src/lib/login/shared/ui/logo/logo.component.html (1 hunks)
- packages/desktop-ui-lib/src/lib/login/shared/ui/logo/logo.component.scss (1 hunks)
- packages/desktop-ui-lib/src/lib/login/shared/ui/logo/logo.component.ts (1 hunks)
- packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.html (1 hunks)
- packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.scss (1 hunks)
- packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/desktop-ui-lib/src/lib/login/shared/ui/logo/logo.component.html
- packages/desktop-ui-lib/src/lib/login/shared/ui/logo/logo.component.scss
🧰 Additional context used
🔇 Additional comments (18)
packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.scss (3)
1-1: LGTM: Good use of shared stylesImporting styles from a shared reusable stylesheet is a good practice. It promotes consistency across components and reduces code duplication.
1-10: Overall: Well-structured SCSS with minor improvement suggestionsThis SCSS file for the social links component is well-organized and makes good use of SCSS features like nesting and mixins. It promotes code reuse by importing shared styles and using a mixin for common social link styles. The suggestions provided are minor and aimed at improving maintainability through the use of variables for specific values. Overall, the file adheres to good SCSS practices and provides a solid foundation for styling the social links component.
5-5: LGTM: Good use of mixin, verify implementationThe use of the
social-links-stylemixin is a good practice for reusing styles. However, ensure that the mixin is properly implemented in the shared stylesheet.To verify the mixin implementation, you can run the following command:
This will help ensure that the mixin is correctly defined and contains the expected styles.
packages/desktop-ui-lib/src/lib/login/shared/ui/logo/logo.component.ts (2)
1-3: LGTM: Imports are appropriate and well-structured.The imports cover all necessary Angular modules and local constants required for the component's functionality.
5-9: LGTM: Component decorator follows Angular best practices.The component is well-defined with a clear selector and separate files for template and styles, which promotes better organization and maintainability.
packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.html (1)
1-2: LGTM: Proper use of async pipe and conditional rendering.The use of the async pipe with
socialLinks$and the subsequent check for non-emptysocialLinksis a good practice. It ensures that the component only renders when data is available, preventing unnecessary DOM elements when there are no social links.packages/desktop-ui-lib/src/lib/login/login.module.ts (3)
17-17: LGTM: SwitchThemeModule import addedThe addition of the SwitchThemeModule import suggests the introduction of theme-switching functionality to the login module. This is a good enhancement for user experience.
19-20: LGTM: New component imports addedThe imports for LogoComponent and SocialLinksComponent indicate a good separation of concerns, likely extracting these elements from the main login component for better modularity and reusability.
36-37: LGTM: Module configuration updated correctlyThe changes to the @NgModule decorator are correct:
- SwitchThemeModule is properly added to the imports array.
- LogoComponent and SocialLinksComponent are correctly added to the declarations array.
- The use of LanguageModule.forChild() is a good practice for feature modules in Angular.
These updates ensure that the new components and functionality are properly integrated into the module.
Also applies to: 39-39
packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.ts (2)
6-20: Well-structured and documented interfaceThe
ISocialLinkinterface is well-defined with clear property types and good use of optional properties. The JSDoc comments for each property enhance code readability and maintainability.
22-27: LGTM: Proper use of decorators and component metadataThe
@UntilDestroy({ checkProperties: true })decorator is correctly applied, which will help prevent memory leaks by automatically unsubscribing from observables. The@Componentdecorator is properly configured with the correct selector, template, and styles.packages/desktop-ui-lib/src/lib/login/login.component.ts (2)
Line range hint
1-41: Consider adding method comments and review remaining functionalityThe core structure of the
NgxLoginComponentremains largely unchanged. However, given the removal of some functionality, it would be beneficial to:
- Add comments to explain the purpose of each method, especially
ngOnInit,forgot, andregister.- Review if the
showPasswordproperty is still being used as intended.- Ensure that the removal of the
logoUrlgetter doesn't affect the component's template or any other parts of the application that might have depended on it.Consider adding method comments like this:
/** * Initializes the component. * Sets up language services for the electron app. */ ngOnInit() { this.languageElectronService.initialize<void>(); } /** * Opens the forgot password page in the default browser. */ public forgot(): void { this.electronService.shell.openExternal('https://app.gauzy.co/#/auth/request-password'); } /** * Opens the registration page in the default browser. */ public register(): void { this.electronService.shell.openExternal('https://app.gauzy.co/#/auth/register'); }To ensure the
showPasswordproperty is still in use, run the following script:#!/bin/bash # Description: Check for showPassword usage in the component's template echo "Checking for showPassword usage in the template:" rg --type html 'showPassword' packages/desktop-ui-lib/src/lib/login/login.component.html
24-27: Verify the impact of removed constructor parametersThe constructor has been simplified by removing
DomSanitizerandGAUZY_ENVparameters. This change suggests that functionality related to URL sanitization and environment-specific configurations has been removed or relocated.Please ensure that:
- The removed functionality (if still needed) is properly handled elsewhere in the application.
- There are no remaining references to
this._domSanitizerorthis._environmentwithin the component.- The logo handling (previously done via the removed
logoUrlgetter) is appropriately managed in the new implementation.Run the following script to verify there are no lingering references:
✅ Verification successful
Constructor parameters have been successfully simplified
The constructor in
login.component.tsno longer references_domSanitizeror_environment. Verified that there are no lingering references within the component.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to removed dependencies # Test 1: Search for _domSanitizer usage echo "Checking for _domSanitizer usage:" rg --type typescript '_domSanitizer' packages/desktop-ui-lib/src/lib/login # Test 2: Search for _environment usage echo "Checking for _environment usage:" rg --type typescript '_environment' packages/desktop-ui-lib/src/lib/login # Test 3: Search for logoUrl references echo "Checking for logoUrl references:" rg --type typescript 'logoUrl' packages/desktop-ui-lib/src/lib/loginLength of output: 988
Script:
#!/bin/bash # Description: Check for any remaining references to removed dependencies # Test 1: Search for _domSanitizer usage echo "Checking for _domSanitizer usage:" rg --type ts '_domSanitizer' packages/desktop-ui-lib/src/lib/login # Test 2: Search for _environment usage echo "Checking for _environment usage:" rg --type ts '_environment' packages/desktop-ui-lib/src/lib/login # Test 3: Search for logoUrl references echo "Checking for logoUrl references:" rg --type ts 'logoUrl' packages/desktop-ui-lib/src/lib/loginLength of output: 1087
packages/desktop-ui-lib/src/lib/login/login.component.scss (2)
30-30: LGTM: Improved responsiveness for SVG wrapperSetting the width to 100% for the
.svg-wrapperclass is a good practice for responsive design. This change ensures that the SVG content adjusts properly to different screen sizes.
Line range hint
1-478: Overall: Improved responsiveness and mobile layoutThe changes in this file significantly enhance the login component's responsiveness and mobile layout. Key improvements include:
- Better use of relative units (rem) for padding.
- Improved centering techniques for mobile elements.
- Adjustments to element widths and positioning for better adaptability.
These modifications will result in a more consistent and user-friendly experience across various device sizes. The suggestions provided in the review comments can further refine the responsive behavior.
packages/desktop-ui-lib/src/lib/login/login.component.html (3)
4-5: Components<gauzy-logo>and<gauzy-switch-theme>are properly integratedThe inclusion of
<gauzy-logo>and<gauzy-switch-theme>components enhances modularity and allows for reusability across the application.
136-136: Component<ngx-social-links>is correctly utilizedReplacing the social links section with
<ngx-social-links>component streamlines the code and promotes reusability.
138-143: New workspace sign-in section is well implementedThe addition of the workspace sign-in section enhances user navigation and aligns with the application's functionality.
packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.html
Show resolved
Hide resolved
packages/desktop-ui-lib/src/lib/login/shared/ui/social-links/social-links.component.html
Show resolved
Hide resolved
☁️ Nx Cloud ReportCI is running/has finished running commands for commit aff5409. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution
✅ Successfully ran 4 targetsSent with 💌 from NxCloud. |
| @@ -42,8 +37,4 @@ export class NgxLoginComponent extends NbLoginComponent implements OnInit { | |||
| public register(): void { | |||
| this.electronService.shell.openExternal('https://app.gauzy.co/#/auth/register'); | |||
There was a problem hiding this comment.
@adkif we should have this URL read from env var, we can't hardcode it, please fix in next PR :) (needed for customizations)
| @@ -17,3 +17,11 @@ export const injector = Injector.create({ | |||
| export const API_ACTIVITY_WATCH_PREFIX = '/buckets'; | |||
|
|
|||
| export const AUTO_REFRESH_DELAY = 60 * 10 * 1000; // milliseconds | |||
|
|
|||
| export const patterns = { | |||
| websiteUrl: /^((?:https?:\/\/)[^./]+(?:\.[^./]+)+(?:\/.*)?)$/, | |||
There was a problem hiding this comment.
@adkif We already have such patterns in @gauzy/ui-core package. Why we are duplicating many things?
* feat: export social links * feat: create a shared logo component * feat: create a shared social links component * feat: add reusable patterns to constants * feat: add link to magic workspace signin and use reusable logo, links and switch * feat: SwitchThemeModule, LogoComponent, and SocialLinksComponent to NgxLoginModule
* fix: #8339 employee & organization parameters to URLs * fix: #8339 project parameters to URLs * fix: #8339 date range picker parameters to URLs * fix: #8339 project parameters to URLs * fix: don't merge new config with old default config * fix: don't merge new config with old default config * fix: organization controller and module improvement * feat: add standardWorkHoursPerDay column to organization entity * feat: #8341 add standardWorkHoursPerDay column migration to organization entity * feat: #8341 add standardWorkHoursPerDay column migration to organization table * feat: #8341 add standardWorkHoursPerDay column migration to tenant table * fix: create/update standard work hours per day * feat: get task by view query model * feat: get task by view query DTO * feat: get tasks by view filter API * fix: save standard work hours per day for organization * fix: get tasks by view filters * fix: remove unused DTO * chore(deps): add chartjs-plugin-annotation chore: add chartjs-plugin-annotation package for annotation support in charts * fix: organization ID filter in query * feat: add translations for Standard Work Hours * feat: #8341 added horizontal dotted line * fix(deepscan): removed unused import * fix(coderrabitai): improve types and finders * fix: sprint DELETE role permission * fix: improve sprints role permissions * fix: #8340 clear data before loading on report pages * fix: issue type value using enum * fix(deepscan): removed unused import * feat: #8339 bookmark query params builder resolver * [Feat] Add new workspace's methods to desktop authentication service (#8375) * feat: add new methods to service * feat: remove unecessary try-catch blocks and type annotations for return values in AuthService methods. * [Feat] Create Reusable Component and Add Workspace Links (#8383) * feat: export social links * feat: create a shared logo component * feat: create a shared social links component * feat: add reusable patterns to constants * feat: add link to magic workspace signin and use reusable logo, links and switch * feat: SwitchThemeModule, LogoComponent, and SocialLinksComponent to NgxLoginModule * fix(cspell): typo spelling :-) * fix: improvement suggested by CodeRabbit * fix: improvement suggested by CodeRabbit * fix: improvement suggested by CodeRabbit * fix: task type enum swagger * fix: issue type enum * feat: #8386 add module and entity for global logging of API * feat: #8386 add repository and subscriber for global logging of API * feat: #8386 table migration for "api_call_log" table * fix: #8386 circular dependency injection in repository * [Fix] Edit team functionality not working * feat: #8386 retrieves call logs API * fix(deepscan): property is accessed without null check * fix: missing role-permission module * fix: better encapsulation and reducing coupling between modules * fix: task view not found update command handler * [Fix] Filters not working * fix: #8386 added origin into `api_call_log` table * [Feat] Implement magic login (#8387) * feat: create reusable workspace selection component * feat: create debounce click directive * feat: create reusable avatar component * feat: implement login magic component * feat: implement login magic workspace component * feat: implement login workspace component * feat: reuse shared core styles * feat: add new component to login * feat: create authentication routes * feat: use lazy loading for authentications routes * fix: add code rabbit suggestions * fix: apply code rabbit suggestions * fix: apply code rabbit suggestions * [Fix] Task deselect on refresh (#8392) * fix: task deselect on refresh * fix: add `organizationTeamId` missing property to time tracker API request and spread `arg.timer` into the request object. * fix: apply code rabbit suggestion * [Fix] Edit contact for (client,customers,leads) functionality not working (#8393) * [Fix] Edit contact for (client,customers,leads) functionality not working * Remove member deletion * [Fix] Remote timer logic stop calls (#8394) * fix: remote timer logic and stopTimer calls * refactor: remote timer logic * [Feat] Resource Link Entity (#8397) * feat: add resource link model and entity * feat: resource link migration * [Feat] Resource Link API (#8399) * feat: add resource link model and entity * feat: resource link migration * feat: add resource link service * feat: resource link commands * feat: add resource link handlers * feat: add resource link subscriber * feat: add resource link controller and DTO * chore: desktop builds on Windows --------- Co-authored-by: Rahul R. <rahulrathore576@gmail.com> Co-authored-by: GloireMutaliko21 <mufunyig@gmail.com> Co-authored-by: Kifungo A <45813955+adkif@users.noreply.github.com> Co-authored-by: samuelmbabhazi <samuelmbabhazi@gmail.com> Co-authored-by: samuel mbabhazi <111171386+samuelmbabhazi@users.noreply.github.com> Co-authored-by: Gloire Mutaliko (Salva) <86450367+GloireMutaliko21@users.noreply.github.com>
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by CodeRabbit
Release Notes
New Features
patternsconstant for input validation.<gauzy-logo>) and theme switching (<gauzy-switch-theme>).Improvements
Bug Fixes
Styling