Skip to content
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

[Brain management] Access management tab #754

Merged
merged 1 commit into from
Jul 25, 2023
Merged

Conversation

mamadoudicko
Copy link
Contributor

@mamadoudicko mamadoudicko commented Jul 24, 2023

Description

  • Add brain access management tab to brain management page
Screen.Recording.2023-07-24.at.17.19.55.mov

@vercel
Copy link

vercel bot commented Jul 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2023 8:09am
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2023 8:09am

@github-actions
Copy link
Contributor

github-actions bot commented Jul 24, 2023

LOGAF Level 3 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/BrainManagementTabs.tsx

The code is generally good, but there are areas for potential improvement. The CSS classes are hard-coded into the component, which could make it difficult to reuse this component with different styles in the future. Consider passing in className as a prop and using the classnames library to combine the passed-in classes with the default ones.

export const BrainManagementTabs = ({ className }: { className?: string }): JSX.Element => {
  // ...
  return (
    <Root
      className={classnames("shadow-md min-h-[50%] dark:shadow-primary/25 hover:shadow-xl transition-shadow rounded-xl overflow-hidden bg-white dark:bg-black border border-black/10 dark:border-white/25 p-4 pt-10", className)}
      defaultValue="settings"
    >
      // ...
    </Root>
  );
};

LOGAF Level 3 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/components/PeopleTab/PeopleTab.tsx

The code is generally good, but there are areas for potential improvement. The form submission logic is directly inside the onSubmit prop. Consider moving this logic to a separate function for better readability and maintainability.

export const PeopleTab = ({ brainId }: ShareBrainModalProps): JSX.Element => {
  // ...
  const handleSubmit = (event: React.FormEvent) => {
    event.preventDefault();
    void inviteUsers();
  };

  return (
    <>
      <form onSubmit={handleSubmit}>
        // ...
      </form>
      // ...
    </>
  );
};

LOGAF Level 4 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/components/PeopleTab/index.ts

LOGAF Level 4 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/components/index.ts

LOGAF Level 3 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/BrainManagementTabs/hooks/useBrainManagementTabs.ts

The code is generally good, but there are areas for potential improvement. The function useBrainManagementTabs is not typed. Consider adding a return type to this function for better type safety.

export const useBrainManagementTabs = (): { selectedTab: BrainManagementTab; setSelectedTab: React.Dispatch<React.SetStateAction<BrainManagementTab>>; brainId: UUID | undefined } => {
  // ...
};

LOGAF Level 4 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/components/index.ts

LOGAF Level 3 - /home/runner/work/quivr/quivr/frontend/app/brains-management/[brainId]/page.tsx

The code is generally good, but there are areas for potential improvement. The error message "Select a brain please" could be more descriptive. Consider changing it to something like "No brain selected. Please select a brain to manage."

if (brainId === undefined) {
  return (
    <div className="flex justify-center mt-5 w-full">
      <div className="bg-blue-100 border border-blue-400 text-blue-700 px-4 py-3 rounded relative max-w-md h-fit">
        <p>No brain selected. Please select a brain to manage.</p>
      </div>
    </div>
  );
}

LOGAF Level 3 - /home/runner/work/quivr/quivr/frontend/lib/components/NavBar/components/NavItems/components/BrainsDropDown/components/BrainActions/components/ShareBrain/ShareBrain.tsx

The code is generally good, but there are areas for potential improvement. The form submission logic is directly inside the onSubmit prop. Consider moving this logic to a separate function for better readability and maintainability.

export const ShareBrain = ({ brainId, name }: ShareBrainModalProps): JSX.Element => {
  // ...
  const handleSubmit = (event: React.FormEvent) => {
    event.preventDefault();
    void inviteUsers();
  };

  return (
    <Modal
      // ...
    >
      <form onSubmit={handleSubmit}>
        // ...
      </form>
      // ...
    </Modal>
  );
};

📝🔄🔍


Powered by Code Review GPT

export const BrainManagementTabs = (): JSX.Element => {
const { selectedTab, setSelectedTab, brainId } = useBrainManagementTabs();

if (brainId === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return a message to invite user to create a brain

}: BrainTabTriggerProps): JSX.Element => {
return (
<Trigger
className={`tracking-wide flex-1 text-lg align-center ${
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add style files in the future ?

gozineb
gozineb previously approved these changes Jul 25, 2023
@gozineb gozineb merged commit 10b0cce into main Jul 25, 2023
3 checks passed
StanGirard pushed a commit that referenced this pull request Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants