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

new exm feed #4967

Merged
merged 10 commits into from
Feb 16, 2024
Merged

new exm feed #4967

merged 10 commits into from
Feb 16, 2024

Conversation

ariunzayarin
Copy link
Collaborator

No description provided.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

PR Type: Enhancement

PR Summary: This pull request introduces a significant refactor and enhancement to the feed component in the exm-web application. It simplifies the PostItem component by removing unused imports and dynamic imports for forms, indicating a shift towards a more static import strategy. Additionally, it introduces new components like FeedComment, FeedDetail, EmojiCount, PostFooter, and PostHeader, aiming to modularize the feed functionality and improve maintainability. Changes to the Sidebar component suggest a reorganization of navigation items, potentially enhancing user navigation experience. The addition of styles for DragNDrop functionality indicates an enhancement in the file upload user interface.

Decision: Comment

📝 Type: 'Enhancement' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
  • Big diff: the diff is too large to approve with confidence.

General suggestions:

  • Ensure that the removal of dynamic imports for form components does not adversely affect the application's performance, particularly in scenarios where these forms are not always needed.
  • Verify that the commented-out navigation logic in the Sidebar component does not negatively impact the user experience, and consider implementing an alternative approach if necessary to maintain or enhance navigation functionality.
  • Review the new modular components introduced for managing feed details, comments, and reactions to ensure they integrate seamlessly with the existing application architecture and meet the project's standards for code quality and performance.
  • Test the new DragNDrop functionality thoroughly to ensure a smooth and intuitive file upload experience for users.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@@ -1,102 +1,45 @@
"use client"

import { useState } from "react"
import dynamic from "next/dynamic"
Copy link

Choose a reason for hiding this comment

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

question (llm): Removing dynamic imports for components like BravoForm, HolidayForm, and PostForm suggests these forms are no longer conditionally loaded. Ensure this change aligns with the performance optimization goals, especially if these components are heavy and not always needed.

@@ -26,7 +27,7 @@ export const Sidebar = () => {
const { unreadCount, refetch } = useChatNotif()

const handleLink = (href: string) => {
router.replace(`/${href}`)
Copy link

Choose a reason for hiding this comment

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

issue (llm): Commenting out the router.replace call changes the navigation behavior. Ensure that this modification doesn't negatively impact the user's ability to navigate the application as expected.

@@ -117,8 +116,7 @@ const EventForm = ({
const [branchSearchValue, setBranchSearchvalue] = useState("")
const [departmentSearchValue, seDepartmentSearchvalue] = useState("")
const [success, setSuccess] = useState(false)
const [imageUploading, setImageUploading] = useState(false)
const [attachmentUploading, setAttachmentUploading] = useState(false)
const [uploading, setUploading] = useState(false)
Copy link

Choose a reason for hiding this comment

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

suggestion (llm): Consolidating imageUploading and attachmentUploading into a single uploading state simplifies the state management. Ensure that this change adequately represents all upload states without losing specificity where needed.

Copy link

sonarcloud bot commented Feb 16, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
14.4% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

idea Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

@Anu-Ujin Anu-Ujin merged commit c25b5e5 into dev Feb 16, 2024
2 of 4 checks passed
@Anu-Ujin Anu-Ujin deleted the new-exm branch February 16, 2024 12:17
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.

None yet

2 participants