feat: Google drive, OneDrive Connectors #739
Conversation
…g folder_id field
…ing options for shared drives
… folder models for Google Drive integration
…syncing permissions for files.
|
|
We have finished reviewing your PR. We have found no vulnerabilities. Reply to this PR with |
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@changesets/cli@2.27.1, npm/gitmoji-cli@9.2.0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
rflihxyz
added
backend
🔌 Connector
…s (for changes api)
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/src/components/Nav/main-nav.tsx(1 hunks)apps/webapp/src/components/RootLayout/index.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/webapp/src/components/RootLayout/index.tsx
[error] 76-77: Provide a text alternative through the alt, aria-label or aria-labelledby attribute
Meaningful alternative text on elements helps users relying on screen readers to understand content's purpose within a page.
If the content is decorative, redundant, or obscured, consider hiding it from assistive technologies with the aria-hidden attribute.
(lint/a11y/useAltText)
[error] 77-78: Provide a text alternative through the alt, aria-label or aria-labelledby attribute
Meaningful alternative text on elements helps users relying on screen readers to understand content's purpose within a page.
If the content is decorative, redundant, or obscured, consider hiding it from assistive technologies with the aria-hidden attribute.
(lint/a11y/useAltText)
[error] 89-92: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 98-98: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 102-102: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
apps/webapp/src/components/Nav/main-nav.tsx
[error] 56-67: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🔇 Additional comments (3)
apps/webapp/src/components/Nav/main-nav.tsx (3)
5-5: Import usage approved
The introduction of the cn utility function for class name composition remains consistent with its usage across the codebase and helps in maintaining cleaner conditional styling.
8-11: Clear and concise interface definition
Defining NavItem with an optional isExternal property is straightforward and flexible, ensuring future extensibility for additional properties like tracking or instrumentation if necessary.
15-42: Clean navigation item configuration
The static array of items effectively encapsulates the navigation configuration in a single place, including external links via isExternal. This is a good approach to keep the component fully configurable and maintainable.
| <path d="M438.6 105.4c12.5 12.5 12.5 32.8 0 45.3l-256 256c-12.5 12.5-32.8 12.5-45.3 0l-128-128c-12.5-12.5-12.5-32.8 0-45.3s32.8-12.5 45.3 0L160 338.7 393.4 105.4c12.5-12.5 32.8-12.5 45.3 0z"/> | ||
| </svg> | ||
| ) : ( | ||
| <svg width="15" height="15" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg"> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Associate a title or aria-label with the SVG icon.
Like the previous comment, this SVG should define either <title> or an aria-label attribute.
- <svg width="15" height="15" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg">
+ <svg
+ width="15"
+ height="15"
+ viewBox="0 0 15 15"
+ fill="none"
+ xmlns="http://www.w3.org/2000/svg"
+ aria-label="Copy to clipboard icon"
+ >📝 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.
| <svg width="15" height="15" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg"> | |
| <svg | |
| width="15" | |
| height="15" | |
| viewBox="0 0 15 15" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| aria-label="Copy to clipboard icon" | |
| > |
🧰 Tools
🪛 Biome (1.9.4)
[error] 102-102: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
| <TooltipTrigger asChild> | ||
| <Button size="sm" variant="outline"> | ||
| {copiesProjectID ? ( | ||
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 448 512" width="15" height="15" fill="#ffffff"> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Provide a title or aria-label for the SVG icon.
SVG elements should include a <title> or aria-label to ensure that assistive technologies can describe the symbol.
Example:
- <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 448 512" width="15" height="15" fill="#ffffff">
+ <svg
+ xmlns="http://www.w3.org/2000/svg"
+ viewBox="0 0 448 512"
+ width="15"
+ height="15"
+ fill="#ffffff"
+ aria-label="Success checkmark icon"
+ >📝 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.
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 448 512" width="15" height="15" fill="#ffffff"> | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| viewBox="0 0 448 512" | |
| width="15" | |
| height="15" | |
| fill="#ffffff" | |
| aria-label="Success checkmark icon" | |
| > |
🧰 Tools
🪛 Biome (1.9.4)
[error] 98-98: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
| <div | ||
| className="cursor-pointer" | ||
| onClick={handleCopyRight} | ||
| > |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enable keyboard interactions for better accessibility.
Elements with onClick should also handle onKeyDown or onKeyUp to support users navigating via keyboard.
Here’s a possible adjustment:
- <div
- className="cursor-pointer"
- onClick={handleCopyRight}
- >
+ <div
+ className="cursor-pointer"
+ role="button"
+ tabIndex={0}
+ onClick={handleCopyRight}
+ onKeyDown={(e) => {
+ if (e.key === "Enter") {
+ handleCopyRight();
+ }
+ }}
+ >📝 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 | |
| className="cursor-pointer" | |
| onClick={handleCopyRight} | |
| > | |
| <div | |
| className="cursor-pointer" | |
| role="button" | |
| tabIndex={0} | |
| onClick={handleCopyRight} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter") { | |
| handleCopyRight(); | |
| } | |
| }} | |
| > |
🧰 Tools
🪛 Biome (1.9.4)
[error] 89-92: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
| {theme == "light" ? | ||
| <img src="/logo-panora-black.png" className='w-14' /> : | ||
| <img src="/logo-panora-white-hq.png" className='w-14' /> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add descriptive alt text for improved accessibility.
Include an alt or aria-label attribute to describe the purpose or content of each <img> element, allowing screen readers to convey this information to users with visual impairments.
Below is a possible fix:
- {theme == "light" ?
- <img src="/logo-panora-black.png" className='w-14' /> :
- <img src="/logo-panora-white-hq.png" className='w-14' />
- }
+ {theme === "light" ?
+ <img src="/logo-panora-black.png" className="w-14" alt="Panora black logo" /> :
+ <img src="/logo-panora-white-hq.png" className="w-14" alt="Panora white logo" />
+ }📝 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.
| {theme == "light" ? | |
| <img src="/logo-panora-black.png" className='w-14' /> : | |
| <img src="/logo-panora-white-hq.png" className='w-14' /> | |
| {theme === "light" ? | |
| <img src="/logo-panora-black.png" className="w-14" alt="Panora black logo" /> : | |
| <img src="/logo-panora-white-hq.png" className="w-14" alt="Panora white logo" /> | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 76-77: Provide a text alternative through the alt, aria-label or aria-labelledby attribute
Meaningful alternative text on elements helps users relying on screen readers to understand content's purpose within a page.
If the content is decorative, redundant, or obscured, consider hiding it from assistive technologies with the aria-hidden attribute.
(lint/a11y/useAltText)
[error] 77-78: Provide a text alternative through the alt, aria-label or aria-labelledby attribute
Meaningful alternative text on elements helps users relying on screen readers to understand content's purpose within a page.
If the content is decorative, redundant, or obscured, consider hiding it from assistive technologies with the aria-hidden attribute.
(lint/a11y/useAltText)
| <div | ||
| key={item.href} | ||
| onClick={() => onLinkClick(item.href)} | ||
| className={cn( | ||
| "flex items-center rounded-md px-3 py-2 text-sm font-medium cursor-pointer", | ||
| "transition-colors", | ||
| "hover:bg-accent hover:text-accent-foreground", | ||
| pathname === `/${item.href}` | ||
| ? "bg-accent text-accent-foreground" | ||
| : "text-muted-foreground hover:text-primary" | ||
| )} | ||
| > | ||
| <item.icon className="mr-2 h-4 w-4" /> | ||
| <span>{item.title}</span> | ||
| {item.isExternal && <ExternalLink className="ml-2 h-4 w-4" />} | ||
| </div> |
There was a problem hiding this comment.
Address accessibility concerns for keyboard navigation
Currently, the click event is assigned to a <div> without providing keyboard accessibility features. According to most accessibility guidelines and the static analysis hint (lint/a11y/useKeyWithClickEvents), elements that handle onClick should also handle keyboard events (e.g., onKeyDown, onKeyUp, or onKeyPress) or be changed to a more semantically appropriate element such as a <button> or an <a> with href. This ensures that keyboard users and assistive technologies can activate the same behavior.
Below is an example fix using a <button> as the interactive element:
- <div
- key={item.href}
- onClick={() => onLinkClick(item.href)}
- className={cn(
- ...
- )}
- >
+ <button
+ key={item.href}
+ onClick={() => onLinkClick(item.href)}
+ onKeyPress={() => onLinkClick(item.href)}
+ className={cn(
+ ...
+ )}
+ >
<item.icon className="mr-2 h-4 w-4" />
<span>{item.title}</span>
{item.isExternal && <ExternalLink className="ml-2 h-4 w-4" />}
- </div>
+ </button>Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 56-67: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
Summary by CodeRabbit
New Features
GoogledrivePermissionMapperandGoogledriveServicefor managing Google Drive permissions.OnedriveQueueProcessorfor processing OneDrive sync jobs.OnedriveandGoogleDriveservices to enhance internal user and group management.Bug Fixes
Documentation
Style
Tests